Re: [boost] boost signals2 review

Subject: Re: [boost] boost signals2 review
From: Stjepan Rajko (stjepan.rajko_at_[hidden])
Date: 2008-11-11 11:25:45

Thank you, Andy, for the review.

Stjepan

On Mon, Nov 10, 2008 at 9:18 AM, Andrew Webber <andy_at_[hidden]> wrote:
> For this review, I have focused on the usability and documentation of the
> new signals2 library.  I haven't really probed into the implementation and
> so I'm assuming that the code is adequate and mostly bug free.
>
> In general, I think that this library should be accepted as a boost
> library.  Other than a few questions and annoyances, I think the library is
> an extremely welcome addition to the boost libraries.
>
> I think that the interface changes required to guarantee thread safety are
> understandable.  I've seen some discussion on the mailing list about
> retaining the trackable interface for backwards compatibility.  While I
> understand the desire to keep the changes required by switching to signals2
> to a minimum, I think signals2 provides a clean break.  I understand
> boost::signals is going to be maintained for the time being.  This way, if
> people don't want to change, they don't need to for the time being.  If they
> want thread safe signals, they should switch to the new library, its new
> techniques and its clean interface.
>
> Here are my specific comments:
> *   Why the change to boost::optional for signal return values.  Maybe I
> missed a discussion about the motivation on the mailling list, but this is
> an interface change that appears to be unnecessary.
>
> *   I would like to see examples provided to explain the use of
> postconstructable, predestructable, and deconstruct_ptr.  I think a concrete
> example might clear up some people's confusion over when it's appropriate to
> use these.
>
> *  I'd also like to see a concrete example shown of switching out the mutex
> class used.  I think three examples would be best: built-in, dummy_mutex,
> and boost::mutex.
>
> *  I'm not very happy about the tendancy of the slot disconnect exceptions
> to show up in Visual Studio output.  For instance, I built a test program
> where several tracked objects were destroyed from different threads, based
> on a random timer, while the slot was being invoked over and over.  The
> program worked well except for a great deal of repeated output like the
> following:
>
> First-chance exception at 0x7c812a5b in SignalsTest.exe: Microsoft C++
> exception:
> boost::exception_detail::clone_impl<boost::exception_detail::error_info_injector<boost::bad_weak_ptr>
>> at memory location 0x0012dc0c..
> First-chance exception at 0x7c812a5b in SignalsTest.exe: Microsoft C++
> exception: boost::signals2::expired_slot at memory location 0x0012df58..
>
>
> I did several experiments with this library on Microsoft Visual Studio
> 2005.  I did not encounter any bugs or problems during my testing.  I have a
> decent amount of experience in this problem domain.  In the past, I have
> implemented a (limited) thread safe wrapper around the boost::signals
> library.  The signals2 library would very effectively replace my wrapper.  I
> spent a couple of hours on my evaluation, I would consider it moderately
> in-depth without evaluating the signals2 implementation.
>
> Best,
>    Andy Webber
>
>