New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make event triggering robust to (un)registration. #9453

Merged
merged 1 commit into from May 12, 2016

Conversation

Projects
None yet
4 participants
@craigcitro
Contributor

craigcitro commented May 11, 2016

Event callbacks can register or unregister new callbacks for the same event
while executing, and the previous triggering implementation allowed for event
callbacks to be inadvertently skipped.

The fix is to make a copy of the list of callbacks before executing any of
them. With this change, the resulting semantics are simple: any callbacks
registered before triggering are executed, and any new callbacks registered
are only visible at the next triggering of the event.

Note that this could potentially break existing callers who expected
newly-appended callbacks were immediately executed.

Fixes #9447. Originally based on a patch by @marksandler2.

PTAL @takluyver

@craigcitro

This comment has been minimized.

Show comment
Hide comment
@craigcitro

craigcitro May 11, 2016

Contributor

I should say -- I don't know if this merits a note in the whatsnew/pr dir, but happy to add one if so.

Contributor

craigcitro commented May 11, 2016

I should say -- I don't know if this merits a note in the whatsnew/pr dir, but happy to add one if so.

@takluyver

This comment has been minimized.

Show comment
Hide comment
@takluyver

takluyver May 11, 2016

Member

Thanks Craig. Yes, it would be good to have a whatsnew note. :-)

Member

takluyver commented May 11, 2016

Thanks Craig. Yes, it would be good to have a whatsnew note. :-)

@minrk minrk added this to the 4.3 milestone May 11, 2016

@craigcitro

This comment has been minimized.

Show comment
Hide comment
@craigcitro

craigcitro May 12, 2016

Contributor

Added a whatsnew note -- PTAL!

Contributor

craigcitro commented May 12, 2016

Added a whatsnew note -- PTAL!

Make event triggering robust to (un)registration.
Event callbacks can register or unregister new callbacks for the same event
while executing, and the previous triggering implementation allowed for event
callbacks to be inadvertently skipped.

The fix is to make a copy of the list of callbacks before executing any of
them. With this change, the resulting semantics are simple: any callbacks
registered before triggering are executed, and any new callbacks registered
are only visible at the next triggering of the event.

Note that this could potentially break existing callers who expected
newly-appended callbacks were immediately executed.

Fixes #9447. Originally based on a patch by @marksandler2.

@minrk minrk merged commit a85ccde into ipython:master May 12, 2016

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@takluyver

This comment has been minimized.

Show comment
Hide comment
@takluyver

takluyver May 12, 2016

Member

Thanks Craig

Member

takluyver commented May 12, 2016

Thanks Craig

@Carreau Carreau modified the milestones: 4.3, 5.0 Jun 3, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment