Re: [Boost-users] [Review] Lockfree review starts today, July 18th

...

...

- fifo::empty is described as non-thread-safe. To what extent? Might it crash the program or corrupt the container, or is it simply that other threads can change the content of the container at a whim, and so the result of calling empty might not persist until the next operation?

it won't crash, but the answer will may be incorrect.

The docs should clarify exactly what can go wrong.

i've already tried to improve the docs (in a separate branch of my git repository)

...

...

- Does the specialization fifo also require that T is trivially assignable, or even that it is assignable at all? It's not clear. If it is actually storing pointers is there a risk of memory leaks if the container is destroyed while non-empty? If it's not actually storing pointers, then what am I to do if I genuinely want a fifo of pointers? Generally I find this specialization a bit worrying since its semantics differ so markedly from the unspecialized equivalent.

i actually like matthew's proposal of templating the dequeue/pop argument. that would make the specialization obsolete. currently, destroying a non-empty fifo would leak memory.

I don't like this. Fundamentally, you must decide whether a container of pointers is responsible for the memory management of the pointers in it. Compare, for example, std::vector and boost::ptr_vector<T>. Your interface is much more akin to the std:: containers, and I think it's wiser to stick to that model. If your users store pointers in it they should concern themselves with the corresponding memory management elsewhere. In particular, the enqueue function should take a T*, not a T as it presently does. Adding half-hearted support to help them will only lead to confusion, obscure memory leaks and problems in the long run.

well, if i remove the pointer specialization and modify pop/dequeue to use a templated argument, there would be a simple requirement that the value type T must be convertible to the dequeue type U (): template <typename T> fifo { template <typename U> bool dequeue (U &); }; for me the main reason for adding a specialization for pointers was to be able to dequeue to smart pointers, which could be achieved with this approach.

...

...

- The fifo and stack insertion functions both return false if "if the freelist is not able to allocate a new fifo node.". Is this only when using a fixed-sized freelist which has used all it's size, or might this also happen with a caching freelist when allocation fails (e.g. due to running out of memory). To put it another way: how are you handling exceptions thrown by the allocator?

no, but i guess for consistency reasons i should do that!

It's not clear that you want to catch the exceptions. Personally I would expect you not to, but the docs should certainly clarify either way. And you should check what guarantees your methods offer in the face of such exceptions, and document them (I imagine the strong guarantee should be no trouble here).

yes, after thinking about it for some time, i actually decided the same way and modified my code accordingly! cheers, tim