Fwd: [signals2][review] little REVIEW

I am forwarding the review below: ---------- Forwarded message ---------- From: <Jean-Christophe.BENOIST@fr.thalesgroup.com> Date: Tue, Nov 4, 2008 at 8:43 AM Subject: [signals2][review] The review of the signals2 library (formerly t hread_safe_signals) begins today, Nov 1st : little REVIEW To: stjepan.rajko@gmail.com (first of all please excuse my weak english, I am not a native english speaker) I am a regular user of boost::signals, so I am very interested by signals2. I'll review from a user standpoint. * On the documentation : - I am surprised to find very little information or rationale about multi-threading in documentation. We know the library is now "thread safe" but we don't know exactly what it means. There is only one MT use case (just) detailed in "tutorial" : what happens when a thread disconnect a slot from a signal, while another thread calls it. But there are another interesting MT use cases : - what if 2 threads simultaneously calls the same signal ? How to mutex slots in this case ? Are slots automatically excluded by an hidden mutex ? - What if a thread adds a slot to a signal while another calls it ? Is the slot added after signal call ? There should be tutorials and explanations for these cases, IMO. - About rationale : why is there special signals2::mutex ? May we use boost::thread::mutex for mutex template ? What are advantages/drawbacks of choosing signals2::mutex or boost::thread::mutex ? - Use of "postconstructible" or "predestructible" classes is rather obscure, and should be tutorialized / better documented. * On interface : - Very close from original signals interface : no particular comment. * On test suite / test cases : - I cannot see any MT test case (from the documentation, in "acceptance tests" section at least). It's rather surprising. I cannot imagine there is no test case for MT (calling a signal while disconnecting slot etc..). In particular, I'd whish to see if this library is robust in a multi-core environment and run test cases in this environment (I know cases where a library is perfectly thread safe on mono-core, even hyperthreading, but fails on multi-core). At least, there should be a rationale explaining why this library should be robust in a multi-core environment (especially with a proprietary signals2::mutex). * Final statement : All in all, I think this library is very close to be accepted as a Boost library, but without MT test cases and MT documentation, I can't give a total agreement for now. I have little doubt this library is MT tested and robust, even in a multi-core environment, but I have no real clue to assert it. Feel free to ask me further details. Best regards Jean-Christophe BENOIST Software Engineer THALES Communications France

On Tuesday 11 November 2008 11:23, Stjepan Rajko wrote:
---------- Forwarded message ---------- From: <Jean-Christophe.BENOIST@fr.thalesgroup.com> - I am surprised to find very little information or rationale about multi-threading in documentation. We know the library is now "thread safe" but we don't know exactly what it means. There is only one MT use case (just) detailed in "tutorial" : what happens when a thread disconnect a slot from a signal, while another thread calls it. But there are another interesting MT use cases : - what if 2 threads simultaneously calls the same signal ? How to mutex slots in this case ? Are slots automatically excluded by an hidden mutex ?
Yes, I'll put something about that when I add the thread-safety section to the design overview. The signal does not hold any locks when it runs a slot. Signals only use mutexes to protect their own data (like the list of connected slots). So a signal invoked concurrently by multiple threads can result in a slot to running concurrently.
- What if a thread adds a slot to a signal while another calls it ? Is the slot added after signal call ?
Once a signal invocation starts running slots, it won't see any new slots connected until the next time it is invoked. It will see disconnections of slots it hasn't reached yet though.
- About rationale : why is there special signals2::mutex ? May we use boost::thread::mutex for mutex template ?
Yes. There is a sentence in the signals2::mutex description that suggests using boost::mutex if you are using Boost.Threads.
What are advantages/drawbacks of choosing signals2::mutex or boost::thread::mutex ?
signals2::mutex is just a header-only mutex.
- Use of "postconstructible" or "predestructible" classes is rather obscure, and should be tutorialized / better documented.
Yes.
* On test suite / test cases :
- I cannot see any MT test case (from the documentation, in "acceptance tests" section at least). It's rather surprising. I cannot imagine there is no test case for MT (calling a signal while disconnecting slot etc..). In particular, I'd whish to see if this library is robust in a multi-core environment and run test cases in this environment (I know cases where a library is perfectly thread safe on mono-core, even hyperthreading, but fails on multi-core). At least, there should be a rationale explaining why this library should be robust in a multi-core environment (especially with a proprietary signals2::mutex).
It's not obvious to me how to write a test for the library's thread-safeness that is any more focused than "we did some signals stuff in multiple threads and the program didn't crash". It's sort of like writing a test for memory leaks. What you need is a debugging tool, like what valgrind's helgrind tries to be. Then you can have your multi-threaded test program, and the test is whether or not the debugging tool detected any funny business (like multiple threads messing with the same variable without locking).

Hi, Frank Mori Hess wrote:
... The signal does not hold any locks when it runs a slot. Signals only use mutexes to protect their own data (like the list of connected slots). So a signal invoked concurrently by multiple threads can result in a slot to running concurrently.
It looks like the implementation of operator(), which emits the signal, locks a mutex, makes a local copy of the state, unlocks the mutex and uses the local copy to emit the signal. What happens if some other thread disconnects after operator() unlocked the mutex, but before the signal is emitted? Will the signal be delivered anyway? Thanks BR, Dmitry
participants (3)
-
Dmitry Goncharov
-
Frank Mori Hess
-
Stjepan Rajko