Segfault when providing a very small initial block_size to constructor of memory pool

[delins]

Even though it doesn't make much sense to do so, if you provide a very small initial block_size argument to memory_pool's constructor, it segfaults. This stripped down version of the example in README.md crashes for me:

#include <iostream>

#include <foonathan/memory/container.hpp> // vector, list, list_node_size
#include <foonathan/memory/memory_pool.hpp> // memory_pool

// alias namespace foonathan::memory as memory for easier access
#include <foonathan/memory/namespace_alias.hpp>


int main()
{
    using namespace memory::literals;

    // a memory pool RawAllocator
    // allocates a memory block - initially 4KiB - and splits it into chunks of list_node_size<int>::value big
    // list_node_size<int>::value is the size of each node of a std::list
    memory::memory_pool<> pool(memory::list_node_size<int>::value, 39); // <- problem here

    // just an alias for std::list<int, memory::std_allocator<int, memory::memory_pool<>>
    // a std::list using a memory_pool
    // std_allocator stores a reference to a RawAllocator and provides the Allocator interface
    memory::list<int, memory::memory_pool<>> list(pool);
    list.push_back(3);
    list.push_back(2);
    list.push_back(1);

    for (auto e : list)
        std::cout << e << ' ';
    std::cout << '\n';
}

If I change the 39 to 40 it's all fine. It seems to be related to the node_size argument: to larger node_size is, the larger block_size needs to be for it not to crash.

Tested with:

• GCC 9.3.0
• Ubuntu 20.04.2
• Ryzen 3950X
• foonathan/memory built in release mode on the same system, commit b4caa03.

When I compiled foonathan/memory in debug mode, the problem is caught in an assert and the following error is thrown:
[foonathan::memory] Assertion failure in function insert_impl (<path>/memory/src/detail/free_list.cpp:527): Assertion "no_nodes > 0" failed.

[MiguelCompany]

That is what min_block_size() is designed to help users with. I guess that memory::memory_pool<>::min_block_size(memory::list_node_size<int>::value, 1) will return 40.

[delins]

Ok, but it's not clear that users must initialize memory_pool with that as the minimum. All the constructor's documentation says is that block_size must be nonzero. Which leads me to believe that if the initial block size would be too small to fit a node, it will would get an additional bigger block, instead of crashing.

[foonathan]

I'll see what I can do to handle it more gracefully.

[delins]

That would be great! Otherwise mentioning it in the documentation would be fine as well I think.

[foonathan]

The behavior is now documented, and I've added more internal assertions in debug mode. The block size of a memory pool must be at least memory_pool<>::min_block_size(node_size, 1), so that it can allocate one node.

Which leads me to believe that if the initial block size would be too small to fit a node, it will would get an additional bigger block, instead of crashing.

That approach doesn't work as the next block size is based on the previous one, so a bigger block might not be big enough, and it needs yet another block etc. It also just wastes memory: the block size that is too small won't ever be used.