Signals don't work properly when they are being disconnected during their processing

[Honza Král]

This happens because disconnect() method does del self.receivers[index] while self.receivers are being iterated over in _live_receivers().

The attached patch has tests and a fix, the tests fail for me without the fix, passes after the fix is applied.

[Honza Král]

Bugs shouold be fixed before release and this is simple and very isolated.

[Gary Wilson]

Honza, I've got a few questions about the use case you mentioned in IRC:

"The use case is that we have a FormField, that stores text in markup in external Model, and passes rendered text in its clean, then registers the post_save hook to save the markup somewhere to enable later editing."

You mention that, as shown in the attached tests, that the second receiver doesn't get run or removed and that this causes problems for the next instance that gets saved.

Well, don't you still have a problem if some other thread saves a different instance of the Model before save gets called on the target instance? In this case, the receivers would get called on the different instance, get removed from the post_save hook, then not get called when the target instance is saved. Or do you not care to be thread safe?

Also, what happens if this custom field cleans fine, but the form still had errors in other fields. In this case the receivers are still connected and could cause havoc on the next instance saved.

If all you are doing is registering receivers and then removing them, could you not simply call the receiver functions after you save the instance and not hook them up to the signal at all? For example, in a post_save method of the field that the form calls after successful validation.

[Honza Král]

Replying to gwilson:

Honza, I've got a few questions about the use case you mentioned in IRC:

​http://github.com/ella/django-markup

...
Well, don't you still have a problem if some other thread saves a different instance of the Model before save gets called on the target instance? In this case, the receivers would get called on the different instance, get removed from the post_save hook, then not get called when the target instance is saved. Or do you not care to be thread safe?

At the moment I don't care about thread safety and this is only my use case (which I agree is not well thought out and still in need of some refinement).

Also, what happens if this custom field cleans fine, but the form still had errors in other fields. In this case the receivers are still connected and could cause havoc on the next instance saved.

yes, it would fail horribly, but that's a bug in my app design

If all you are doing is registering receivers and then removing them, could you not simply call the receiver functions after you save the instance and not hook them up to the signal at all? For example, in a post_save method of the field that the form calls after successful validation.

I wanted to avoid this so that the form doesn't have to be aware of the field and special case it. I wanted to have a generic solution without any other part of the framework having to know anything about it. It is still a work in progress.

But that is just my little twisted use case, consider for example a signal handler for gathering statistics that will disconnect after given amount of time has passed, it would also cause some other signals not being called. The point is disconnecting signals from within signals SHOULD work, and if not, it should raise an exception or do something sensible. Making it work correctly seems to be the easier solution.

[Jacob]

(In [10831]) Fixed #11134: signals recievers that disconnect during their processing no longer mess things up for other handlers. Thanks, Honza Kral.

While I was at it I also cleaned up the formatting of the docstrings a bit.

[Jacob]

(In [10832]) [1.0.X] Fixed #11134: signals recievers that disconnect during their processing no longer mess things up for other handlers. Thanks, Honza Kral. Backport of [10831] from trunk.

[Jacob]

Milestone 1.1 deleted