Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

iteration_allocator issues #31

Closed
twiggler opened this issue Mar 27, 2018 · 9 comments
Closed

iteration_allocator issues #31

twiggler opened this issue Mar 27, 2018 · 9 comments

Comments

@twiggler
Copy link

twiggler commented Mar 27, 2018

This might be me, but when exiting a unit test in my project I get the line:
[foonathan::memory] Allocator foonathan::memory::heap_allocator (at 0000000000000000) leaked 16777248 bytes.

Indeed, I use an iteration_allocator which allocates 16M for the "initial block" using the default_allocator, which is the heap_allocator on my system. I cannot discover where this initial block is supposed to be released, since neither iteration_allocator nor fixed_block_allocator has a user-defined destructor?

Moreover, in release build the unit tests hangs at the very end unless I forcefully terminate with exit(.); might be unrelated though.

foonathan added a commit that referenced this issue Mar 28, 2018
Also remove leak checking completely as it isn't really applicable there. Fixes #31.
foonathan added a commit that referenced this issue Mar 28, 2018
Also remove leak checking completely as it isn't really applicable there. Fixes #31.
@foonathan
Copy link
Owner

Stupid oversight on my part, now fixed.

Moreover, in release build the unit tests hangs at the very end unless I forcefully terminate with exit(.); might be unrelated though.

I can't reproduce that, could you provide more information?

@twiggler
Copy link
Author

Thanks for the fix 👍

I cannot reproduce the test hanging in release build, because it now SEGFAULTS when copying a custom container which uses an iteration_allocator wrapped in an any_std_allocator. I already observed this in the "release with debug information" build before and dismissed it as a fluke, so this new behavior is most likely not caused by your most recent commit.

I will try to isolate the problem.

@twiggler
Copy link
Author

twiggler commented Mar 29, 2018

I've isolated the problem to the following case (with -O2):

auto entityAllocator = iteration_allocator<2, fixed_block_allocator<>>(16 * 1024 * 1024);
std::vector<int, foonathan::memory::any_std_allocator<int, foonathan::memory::no_mutex>> v(10, make_any_std_allocator<char, no_mutex>(entityAllocator));
auto v2(v);
auto v3 = v;

Note that the passed std_allocator needs to be converted from char to int.
If i write this, it runs fine:

auto entityAllocator = iteration_allocator<2, fixed_block_allocator<>>(16 * 1024 * 1024);
std::vector<int, foonathan::memory::any_std_allocator<int, foonathan::memory::no_mutex>> v(10, make_any_std_allocator<int, no_mutex>(entityAllocator));
auto v2(v);
auto v3 = v;

In the second code fragment, the passed std_allocator does not need to be converted. I cannot figure out yet why this is a problem, although perhaps something is referencing the temporary std_allocator that is created when converting.

@foonathan
Copy link
Owner

Thank you I will investigate.

@foonathan foonathan reopened this Mar 29, 2018
@foonathan foonathan changed the title iteration_allocator block deallocation iteration_allocator issues Mar 29, 2018
@twiggler
Copy link
Author

twiggler commented Mar 30, 2018

foonathan added a commit that referenced this issue Apr 2, 2018
foonathan added a commit that referenced this issue Apr 2, 2018
@foonathan
Copy link
Owner

Should be fixed now.

@twiggler
Copy link
Author

twiggler commented Apr 2, 2018

Sweet, the test now runs fine.

Apart from the failure to properly destruct the basic_allocator, I gather from your changes to std_allocator that this constructor did not win the overload resolution; if so, do you know why? Because it takes base_allocator as a const reference?

What techniques did you employ to debug this issue, if I may ask?

@foonathan
Copy link
Owner

I gather from your changes to std_allocator that this constructor did not win the overload resolution; if so, do you know why?

Because that constructor isn't forwarded to the allocator_reference class, so it is not available outside the reference_storage.

What techniques did you employ to debug this issue, if I may ask?

I first did trial and error (which lead to the insertion of destructor calls, because without them the code is technically UB). This actually fixed an even more simplified example, but not yours.

Because it worked fine in debug mode, I used clang's sanitizers there.

The undefined behavior sanitizer discovered the alignment issues in small free list which were completely unrelated but had to be fixed. Then the address sanitizer reported a stack-use-after-scope. With that I could simplify everything to something like this:

any_std_allocator<int> alloc = make_any_std_allocator<char>(heap_allocator{});
alloc.allocate(1);

Because it worked fine with int at both calls, I looked at the converting constructors of std_allocator and realized that they were broken.

@twiggler
Copy link
Author

twiggler commented Apr 2, 2018

Because that constructor isn't forwarded to the allocator_reference class, so it is not available outside the reference_storage.

Ah, that was the piece of the puzzle I was missing.

Thanks for the write-up!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants