talloc: talloc_set_memlimit causes all reallocs to fail when used on pools. talloc_set_memlimit not enforced correctly on pools.
Jeremy Allison
jra at samba.org
Tue Oct 20 19:01:51 UTC 2020
On Tue, Oct 20, 2020 at 01:42:00PM -0500, Arran Cudbard-Bell wrote:
>
>
> > On Oct 20, 2020, at 1:33 PM, Jeremy Allison <jra at samba.org> wrote:
> >
> > On Tue, Oct 20, 2020 at 01:03:14PM -0500, Arran Cudbard-Bell wrote:
> >>
> >> Then there's another issue with object_count ending
> >> up off by one, which means talloc_free_children doesn't
> >> actually return memory to the pool, and that messes up
> >> some of the other tests I'm adding. Just tracking down
> >> when and why this happens now.... It might have been a
> >> pre-existing issue and not related to this patch, I'm just
> >> seeing it because of using talloc_free_children to reset
> >> the pool between some tests.
>
> Apologies, there was a steal I didn't spot in the tests.
> One of the chunks was moved out of the pool into the root ctx.
>
> Explicitly freeing the chunk or stealing it back into the pool
> means talloc_free_children works as expected.
>
> > Oh, I think that may be here:
> >
> > 1894 #if ALWAYS_REALLOC
> > 1895 if (pool_hdr) {
> > 1896 new_ptr = tc_alloc_pool(tc, size + TC_HDR_SIZE, 0);
> > 1897 pool_hdr->object_count--;
> > 1898
> >
> > We don't reset pool_hdr->object_count on early return
> > in this codepath.
> > Are you setting ALWAYS_REALLOC==1 in your tests ?
>
> I wasn't but that does look like a valid issue. We'd end up with
> with too few chunks in the allocation failed due to a memlimit.
Yes, for that codepath we need to move the:
1897 pool_hdr->object_count--;
to the point where we know we're returning a
valid new object and we've successfully done
the memcpy. But that's a different bugfix
I think.
> > Not sure if this should be:
> >
> > 1894 #if (ALWAYS_REALLOC != 0)
>
> That would be consistent with the other check at 1841.
Yeah, again a tidyup for later.
More information about the samba-technical
mailing list