[signals/threadsafe version] Atomic disconnects

[Johan Torp]

I'm wondering if the new thread safe implementation of signals guarantees atomic disconnects.

line1: signal<void(), multi_threaded> sig;
line2: thread t(sig); // Start a new thread which will emit "sig"
line3: sig.connect(&some_func);
line4: sig.disconnect(&some_func);
line5: ...

Am I guaranteed that some_func isn't called at line 5 or afterwards?


If disconnect is blocking, how do you prevent the following dead-lock during signal emission?
- Emitting thread tries to acquire a mutex X via a slot
- Disconnect calling thread is holding X and gets blocked on disconnect


Best Regards, Johan Torp

[Frank Mori Hess]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On Monday 11 February 2008 10:11 am, Johan Torp wrote:

> I'm wondering if the new thread safe implementation of signals guarantees
> atomic disconnects.
>
> line1: signal<void(), multi_threaded> sig;
> line2: thread t(sig); // Start a new thread which will emit "sig"
> line3: sig.connect(&some_func);
> line4: sig.disconnect(&some_func);
> line5: ...
>
> Am I guaranteed that some_func isn't called at line 5 or afterwards?

Sorry it took me so long to reply to this, I just noticed it.  If some_func is
in the process of running in another thread when you call disconnect(), it
may still be running when disconnect() returns.  disconnect() does not block
waiting for any slots to complete.  In fact, no mutexes inside the signal are
held while a slot is executing, to avoid deadlock issues.

> If disconnect is blocking, how do you prevent the following dead-lock
> during signal emission?
> - Emitting thread tries to acquire a mutex X via a slot
> - Disconnect calling thread is holding X and gets blocked on disconnect

- --
Frank
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)

iD8DBQFHzD6K5vihyNWuA4URAnk8AKCvf2Ldcn4QHBWwyCCFVAhSnyPeEQCeIPOK
f2DFJgH45x5lArF1EJPelPw=
=Gq8c
-----END PGP SIGNATURE-----
_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Johan Torp]

Frank Mori Hess wrote:

Sorry it took me so long to reply to this, I just noticed it.  If some_func is
in the process of running in another thread when you call disconnect(), it
may still be running when disconnect() returns.  disconnect() does not block
waiting for any slots to complete.  In fact, no mutexes inside the signal are
held while a slot is executing, to avoid deadlock issues.

This means disconnect semantics are different for threaded and non-threaded policies. How will you make this clear to users? A typical signals n' slots use case  is - at least for me:

signal<void()> sig;

class Foo{
  Foo() {
    con = sig.connect(bind(&Foo::some_func, this));
 }

  scoped_connection con;

  void some_func() {}
};

If the signal was thread-safe this could very well crash the user application.

[Peter Dimov-5]

Johan Torp wrote:
...

> This means disconnect semantics are different for threaded and
> non-threaded policies. How will you make this clear to users? A
> typical signals n' slots use case  is - at least for me:
>
> signal<void()> sig;
>
> class Foo{
>  Foo() {
>    con = sig.connect(bind(&Foo::some_func, this));
> }
>
>  scoped_connection con;
>
>  void some_func() {}
> };
>
> If the signal was thread-safe this could very well crash the user
> application.

It would also crash if the signal wasn't thread safe, so where's the
difference?

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Frank Mori Hess]

On Friday 07 March 2008 02:50 am, Johan Torp wrote:

> A typical signals n' slots
> use case  is - at least for me:
>
> signal<void()> sig;
>
> class Foo{
>   Foo() {
>     con = sig.connect(bind(&Foo::some_func, this));
>  }
>
>   scoped_connection con;
>
>   void some_func() {}
> };

To have the connection disconnect on the Foo object's destruction, you're
supposed to pass a shared_ptr owning the object (either directly or
indirectly) to slot::track() before connecting the slot.  This insures the
object is not destroyed while a slot invocation is in progress (the signal
converts its weak_ptr copy to a shared_ptr while the slot runs), and
disconnects the slot when the tracked weak_ptr expires.  It does have the
drawback that you often can't track connections made in the constructor
though, since enable_shared_from_this doesn't work there.  I did provide
postconstructible/deconstruct_ptr to support postconstructors, although it
does all add up to a bit more typing.

I've attached an altered version of your example which does what I've
described.

--
Frank

[example.cpp]

---

#include <boost/deconstruct_ptr.hpp>
#include <boost/enable_shared_from_this.hpp>
#include <boost/postconstructible.hpp>
#include <boost/shared_ptr.hpp>
#include <boost/thread_safe_signal.hpp>
#include <iostream>

boost::signal<void ()> sig;

class Foo: public boost::enable_shared_from_this<Foo>,
  public boost::postconstructible
{
public:
  static boost::shared_ptr<Foo> create()
  {
    return boost::deconstruct_ptr(new Foo());
  }
  virtual void postconstruct()
  {
    typedef typeof(sig) sig_type;
    sig.connect(
      sig_type::slot_type(&Foo::some_func, this).track(shared_from_this()));
  }
  void some_func()
  {
    std::cout << __PRETTY_FUNCTION__ << std::endl;
    /*...*/
  }
private:
  Foo() {/*...*/}
};

int main()
{
  boost::shared_ptr<Foo> f = Foo::create();
  sig();
  f.reset();
  sig();
  return 0;
}

---

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Johan Torp]

Peter Dimov-5 wrote:

It would also crash if the signal wasn't thread safe, so where's the
difference?

Why would it crash?

Johan

[Johan Torp]

Frank Mori Hess wrote:

To have the connection disconnect on the Foo object's destruction, you're
supposed to pass a shared_ptr owning the object (either directly or
indirectly) to slot::track() before connecting the slot.  This insures the
object is not destroyed while a slot invocation is in progress (the signal
converts its weak_ptr copy to a shared_ptr while the slot runs), and
disconnects the slot when the tracked weak_ptr expires.  It does have the
drawback that you often can't track connections made in the constructor
though, since enable_shared_from_this doesn't work there.  I did provide
postconstructible/deconstruct_ptr to support postconstructors, although it
does all add up to a bit more typing.

Thanks for the clarification. This solution forces the use of shared_ptrs and might keep a Foo instance alive a little bit longer. Especially the latter requirement is a no-no for me.


I created my own thread safe signals implementation which requires a "CallbackRequester" to connect a synchronous slot to an asynchronous signal:

class CallbackRequester { virtual void callLater(const boost::function<void()>& callback) = 0; };

The callbackrequester implementation -typically a queue - switches to the thread which the synchronous slot "belongs" to. This is of course very intrusive to the entire design of an application - callbackrequesters implementations need to exist for all threads and be passed around all over the application. However, it made it possible to implement a safe scoped_connection with RAII semantics. User code look something like this:

AsyncSignal<void()> sig;

class Foo{
Foo(boost::weak_ptr<CallbackRequester> req)
: con(sig, boost::bind(&Foo::SomeFunc, this), req) {}
 
void SomeFunc() { ...}

AsyncSignalConnection con;
};
 

Johan

[Peter Dimov-5]

Johan Torp:

> Peter Dimov-5 wrote:
>>
>> It would also crash if the signal wasn't thread safe, so where's the
>> difference?
>>
>
> Why would it crash?

Because the thread-safe signal will only crash in your program when the
signal is being called in one thread and the class was destroyed in another.
Perhaps I'm missing something though. Your program wasn't complete.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Frank Mori Hess-2]

On Sunday 09 March 2008 05:21, Johan Torp wrote:
> Thanks for the clarification. This solution forces the use of
> shared_ptrs and might keep a Foo instance alive a little bit longer.
> Especially the latter requirement is a no-no for me.

I could add something like a "join()" method to the connection class, which
would block until the associated slot is finished running.  Then you could
just follow your disconnect() call with a join(), if you are sure it won't
result in deadlock.

> The callbackrequester implementation -typically a queue - switches to
> the thread which the synchronous slot "belongs" to. This is of course
> very intrusive to the entire design of an application -

Yes, I don't think this belongs in a signals library.  If you have a
separate event loop/method request framework, you can just implement a
slot which simply queues an event in the desired thread's event queue.  
Or, it would also be very similar to just having your slot send a method
request to an active object (resisting urge to plug my active object lib
once again...).

> callbackrequesters implementations need to exist for all threads and be
> passed around all over the application. However, it made it possible to
> implement a safe scoped_connection with RAII semantics. User code look

--
Frank



_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Peter Dimov-5]

Johan Torp:
...

> Thanks for the clarification. This solution forces the use of shared_ptrs
> and might keep a Foo instance alive a little bit longer. Especially the
> latter requirement is a no-no for me.

If thread A is in the middle of a call to foo.f() and thread B attempts to
destroy foo, your only options are (1) keep foo alive a little bit longer or
(2) crash. Of course I may be missing something.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Johan Torp]

Peter Dimov-5 wrote:

> Thanks for the clarification. This solution forces the use of shared_ptrs
> and might keep a Foo instance alive a little bit longer. Especially the
> latter requirement is a no-no for me.

If thread A is in the middle of a call to foo.f() and thread B attempts to
destroy foo, your only options are (1) keep foo alive a little bit longer or
(2) crash. Of course I may be missing something.

No, I believe you are correct, that's why I chose my own "architecture intrusive" way of implementing thread safe signals.

[Johan Torp]

Frank Mori Hess-2 wrote:

> The callbackrequester implementation -typically a queue - switches to
> the thread which the synchronous slot "belongs" to. This is of course
> very intrusive to the entire design of an application -

Yes, I don't think this belongs in a signals library.  

Absolutely.

However, I think the possibly delayed disconnect semantics are very unintuitive and should be pointed out more than clearly in the documentation.

Johan
 

[Johan Torp]

My bad. I meant it wouldn't crash if it was a single threaded program.

My point is this; Existing boost.signal users who want to use signals across thread boundaries might be fooled to think that they can just change the threading policy of their existing signals and everything works nicely.

Johan

Peter Dimov-5 wrote:

Johan Torp:

> Peter Dimov-5 wrote:
>>
>> It would also crash if the signal wasn't thread safe, so where's the
>> difference?
>>
>
> Why would it crash?

Because the thread-safe signal will only crash in your program when the
signal is being called in one thread and the class was destroyed in another.
Perhaps I'm missing something though. Your program wasn't complete.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost