Is pool::allocate correct?

[angainor]

It seems to me that pool::allocate can return without unlocking an internally locked mutex. There are a number of return statements on the way before m_mutex.unlock();. It also seems there is quite a number of if (m_free_stack.pop(b)) return b; calls. Can this be simplified?

    auto allocate()
    {
        block_type b;
        if (m_free_stack.pop(b)) return b;
        if (!m_mutex.try_lock())
            if (m_free_stack.pop(b)) return b;
        if (m_free_stack.pop(b)) return b;
        for (auto& kvp : m_segments) kvp.first->collect(m_free_stack);
        if (m_free_stack.pop(b)) return b;
        unsigned int counter = 0;
        while (!m_free_stack.pop(b))
        {
            // add segments every 2nd iteration
            if (++counter % 2 == 0) add_segment();
        }
        m_mutex.unlock();
        return b;
    }

[angainor]

It also seems that the function body can be executed with and without a lock:

        if (!m_mutex.try_lock())
            if (m_free_stack.pop(b)) return b;

If m_free_stack.pop fails, the execution continues, but the lock has not been acquired.

[boeschf]

I pushed a fix: added a while loop to lock for sure. Now it should be thread safe. About simplification: we could potentially remove some of the pops, yes