in addition to not being exception safe, all of the explicit entry and early exit from the mutex is a fully loaded footgun for some future developer working on this code. RAII is a core idiom of C++ and scoped locks/guards are the only sensible and safe way to write this sort of code so that will be both exception safe and have a chance of not biting someone down the road.
Also, std::condition_variable has explicit support for handling spurious wakeups. see - http://en.cppreference.com/w/cpp/thread/condition_variable/wait The second variant of wait() takes a predicate which can be used to ignore spurious wakeups. On Sat, Feb 21, 2015 at 10:51 AM, Thomas Rodgers <rodg...@twrodgers.com> wrote: > s/comdition/condition/ > > On Sat, Feb 21, 2015 at 10:45 AM, Thomas Rodgers <rodg...@twrodgers.com> > wrote: > >> Which all brings us to my next question - >> >> Can we just move on to -std=c++11 for future libzmq versions? The big 3 >> compilers (well mostly, Microsoft still presents a few issues) support >> C++11 at this point. >> >> Many of the issues below would just 'go away' with the use of std::mutex, >> std::unique_lock, and std::comdition_variable. >> >> >> On Saturday, February 21, 2015, Bjorn Reese <bre...@mail1.stofanet.dk> >> wrote: >> >>> On 02/21/2015 04:44 PM, Doron Somech wrote: >>> >>> > (https://github.com/zeromq/libzmq/blob/master/src/mailbox_safe.hpp) >>> have >>> >>> I had a quick look at this class... >>> >>> The workaround in the destructor is not thread-safe. Another thread >>> may enter and wait between the sync->unlock() and the end of the >>> destructor scope. You need to add a state variable to prevent this >>> from happening. >>> >>> Furthermore, if another thread is waiting on the condition variable, >>> then the mutex is unlocked while it is waiting. You need to >>> notify (broadcast) the condition variable to wake up the other thread >>> and get it out of the class. You will most likely need to to add a >>> reference count to keep track of how many pending threads are waiting >>> in order to know when it is safe to exit the destructor. >>> >>> The use of sync->lock() and sync->unlock() is not exception safe. I >>> suggest that you use a scoped lock instead of the explicit calls. Read >>> this for inspiration: >>> >>> http://en.cppreference.com/w/cpp/thread/unique_lock >>> >>> The code does not handle spurious wakeups from the condition variable. >>> >>> _______________________________________________ >>> zeromq-dev mailing list >>> zeromq-dev@lists.zeromq.org >>> http://lists.zeromq.org/mailman/listinfo/zeromq-dev >>> >> >
_______________________________________________ zeromq-dev mailing list zeromq-dev@lists.zeromq.org http://lists.zeromq.org/mailman/listinfo/zeromq-dev
