Opened 9 years ago

Closed 9 years ago

#9103 closed Bugs (fixed)

execution of slots on deletion of signals

Reported by: wim@… Owned by: Frank Mori Hess
Milestone: Boost 1.56.0 Component: signals2
Version: Boost 1.54.0 Severity: Problem
Keywords: Cc:

Description

This code:

struct A { public:

A() {

sig.connect(0, boost::bind(&A::a, this)); sig.connect(1, &A::b);

} void a() {delete this;} static void b() {std::cout << "hello\n";} boost::signals2::signal<void(void)> sig;

}; A *a = new A; a->sig();

Does not print 'hello'.

Yet, the documentation of boost.signals2 states:

Signal/slot disconnections occur when any of these conditions occur:

(1) The connection is explicitly disconnected via the connection's disconnect method directly, or indirectly via the signal's disconnect method, or scoped_connection's destructor.

(2) An object tracked by the slot is destroyed.

(3) The signal is destroyed.

[Note: we're in case (3)]

These events can occur at any time without disrupting a signal's calling sequence. If a signal/slot connection is disconnected at any time during a signal's calling sequence, the calling sequence will still continue but will not invoke the disconnected slot. Additionally, a signal may be destroyed while it is in a calling sequence, and which case it will complete its slot call sequence but may not be accessed directly.

Since the slot is deleted in the calling sequence, I expect the calling sequence to continue, and 'hello' should be printed. This behaviour would be consistent with boost.signals (the deprecated version).

Change History (7)

comment:1 by anonymous, 9 years ago

Component: Nonesignals2
Owner: set to Frank Mori Hess

in reply to:  description comment:2 by Frank Mori Hess, 9 years ago

Replying to wim@…:

These events can occur at any time without disrupting a signal's calling sequence. If a signal/slot connection is disconnected at any time during a signal's calling sequence, the calling sequence will still continue but will not invoke the disconnected slot. Additionally, a signal may be destroyed while it is in a calling sequence, and which case it will complete its slot call sequence but may not be accessed directly.

Since the slot is deleted in the calling sequence, I expect the calling sequence to continue, and 'hello' should be printed. This behaviour would be consistent with boost.signals (the deprecated version).

The docs are contradictory here. The signal destructor is documented in the reference to disconnect all slots. A disconnection in Signals2 takes effect for all slots which have not yet begun to be executed (even if earlier slots in an invocation have already run when the disconnection happens). This is a different behavior from the original Signals.

So the options as I see them are:

  1. Delete the sentence describing a signal being destroyed in its calling sequence. Technically, the slot call sequence does complete but since all the slots were disconnected by the signal destructor, no further slots are executed. Also, there should be a paragraph added to the "Porting" section of the documentation noting the changed behavior of disconnect from the original Signals.
  2. Delete the sentence describing the signal destructor as disconnecting all connected slots. Instead, the slots would be disconnected at some later point once all concurrent signal invocations are complete. It would be possible for a series of overlapping signal invocations (via the weak references to the destroyed signal's implementation used by signals-used-as-slots) to prevent the slots from ever being disconnected. The implementation also adds a slight overhead of once atomic increment and one decrement per signal invocation, for the reference counting to determine when the disconnects should be finally performed.
  3. Make Signals2 disconnection behave consistently with the original Signals. Every signal invocation would have to make a local copy of the disconnected/blocked state for all the slots before executing any of them, and then ignore any changes that occur during invocation. This may cause issues with existing Signals2 code, and it's not clear to me that this behavior is actually more desirable than the existing behavior. Although, it would ease porting for those who haven't already. It would also be make disconnects more symmetrical with connects, since newly connected slots are ignored by signals2 signal invocations if the invocation has already begun executing slots.

I'm currently inclined to go with option 1.

comment:3 by wim@…, 9 years ago

The question to answer is: how do you want to support signal/slot connections where slots may modify the data structure in such a way that the signalling object is deleted.

In Wt (www.webtoolkit.eu) we regularly encounter these 'delete this' cases (directly or indirectly), e.g. in situations where the 'Ok' button of a dialog or a widget causes the dialog or widget to be closed or removed. With signals2, the behaviour changed when multiple slots are connected to such a self terminating signal, which lead to a bug report. In this report, a signal from a single-shot timer was connected to a method that executed the 'delete this', in addition to multiple connections that are handled by other objects that are not affected by this deletion. That's just another example of a self-destructing signal.

I'd prefer compatibility with boost.signals is implemented. As you indicate, it makes the signals disconnect behaviour more consistent with the connect case, and it would also make signals2 more compatible with the original signals library. Projects moving from signals to signals2 (because of the official deprecation of the original signals library) will run in this subtle, difficult to debug use case. As I understand it, the boost.signals behaviour is slightly different from what you describe in (3): disconnected slots will not be executed when disconnects occur during the calling sequence. I have no idea what happens with blocked slots.

Or can you suggest a work-around (copying the signal to the stack before invoking it?) that would have compatible behaviour?

in reply to:  3 comment:4 by Frank Mori Hess, 9 years ago

Replying to wim@…:

In Wt (www.webtoolkit.eu) we regularly encounter these 'delete this' cases (directly or indirectly), e.g. in situations where the 'Ok' button of a dialog or a widget causes the dialog or widget to be closed or removed. With signals2, the behaviour changed when multiple slots are connected to such a self terminating signal, which lead to a bug report. In this report, a signal from a single-shot timer was connected to a method that executed the 'delete this', in addition to multiple connections that are handled by other objects that are not affected by this deletion. That's just another example of a self-destructing signal.

Thanks for clarifying your use case. I see how the disconnect in the signal destructor is annoying for you.

As I understand it, the boost.signals behaviour is slightly different from what you describe in (3): disconnected slots will not be executed when disconnects occur during the calling sequence. I have no idea what happens with blocked slots.

Thanks for pointing that out. I'll play around with the original Signals library when I get the time, to refresh my memory of its behavior. Maybe the most compatible and easiest change will turn out to be to just drop the call to disconnect_all_slots in the Signals2 signal destructor.

Or can you suggest a work-around (copying the signal to the stack before invoking it?) that would have compatible behaviour?

signals are non-copyable so you can't do that. If your signal was owned by shared_ptr, you could prevent the signal's destruction during invocation by copying an owning shared_ptr to the stack.

comment:5 by Frank Mori Hess, 9 years ago

(In [85836]) Don't force disconnection of all slots in signal destructor. Refs #9103

comment:6 by Frank Mori Hess, 9 years ago

Milestone: To Be DeterminedBoost 1.56.0
Status: newassigned

Too late to merge into release for 1.55, change will be in 1.56.

comment:7 by Frank Mori Hess, 9 years ago

Resolution: fixed
Status: assignedclosed

(In [86632]) Merge from trunk. Fixes #9103

Note: See TracTickets for help on using tickets.