Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

The code in listings 3 and 5 is not thread safe. The list could easily become empty between the call to empty() and acquiring the mutex, leading to a race condition. I see two other subtle race conditions as well. I haven't looked at the rest but this density of bugs is enough to convince me that this author doesn't understand multithreaded programming well enough to give advice on the topic.

Other bugs:

1. Assumes that std::list<T>::empty() is an atomic operation.

2. Assumes it's safe to use separate locks for reading and writing, which std::list says nothing about. It probably screws up even in practice for at least the case where you have only one element in the list and two threads execute push() and pop() at the same time.




Agreed. It's better idea to use mutex + 2 semaphores to implement queue with blocking: http://en.wikipedia.org/wiki/Producer-consumer_problem#Using...

(it's a classical computer science problem with classical solution)


In fairness to the rest of the article, in Listing 9 the author does protect the call to empty() with (the same) lock and also moves the condition inside of a while loop.


I agree. It looks like their read implementation means that every thread pops the same item off as well, since they clone to a temp variable.


Threads have separate stacks and the call to front() and pop_front() is protected by the mutex, so that part is fine as far as I can see.


I'm not familiar with the STL implementation, but it seems the pop would need to be coherent with the push in an empty stack. Like if a thread was trying to pop the only entry while another was trying to push, 'intuitively' there'd be a race condition there if STL isn't thread safe (which is the whole point of the article I think).


I agree. Listing 11 won't even compile as was_empty is declared const.


That use of const is fine, as it's never modified after the initial value is assigned.


No, Jabbles is correct: look at listing 11. It's reassigned five lines down.


Oops, I was looking at listing 8. Thanks.




Consider applying for YC's Fall 2025 batch! Applications are open till Aug 4

Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: