Skip to content

Allow threads during macos event loop #27755

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Feb 8, 2024
Merged

Allow threads during macos event loop #27755

merged 1 commit into from
Feb 8, 2024
+4 −0

Conversation

ksunden
Copy link
Member

@ksunden ksunden commented Feb 7, 2024

PR summary

Add START/END "ALLOW THREADS" macros to the event loop code.
This breaks the deadlock, which allows the GIL to be acquired in an alternate thread.

It is not clear to me why this is MacOS version dependent, but this does appear to work and seems sensible to me.

Closes #27720

PR checklist

@QuLogic QuLogic added this to the v3.8.3 milestone Feb 7, 2024
@tacaswell
Copy link
Member

I had a 14.1 system, confirmed the code in #27720 worked as expected, then updating the mac to 14.3 it failed and then using this branch it works again.

@QuLogic QuLogic requested a review from greglucas February 7, 2024 22:03
greglucas
greglucas approved these changes Feb 8, 2024
View reviewed changes
Copy link
Contributor

@greglucas greglucas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this makes sense. We have two separate entry-points to the runloops.

  1. Through pyplot.show() which starts the mainloop and has an [NSApp run] that is surrounded by the ALLOW_THREADS calls:

    matplotlib/lib/matplotlib/backends/backend_macosx.py

    Lines 180 to 183 in c2aa4ee

    def start_main_loop(cls):
    # Set up a SIGINT handler to allow terminating a plot via CTRL-C.
    with _allow_interrupt_macos():
    _macosx.show()
  2. Through pyplot.pause() which starts an eventloop (not necessarily the mainloop), which is not surrounded by the ALLOW_THREADS currently and this PR implements:

    matplotlib/lib/matplotlib/backends/backend_macosx.py

    Lines 113 to 117 in c2aa4ee

    def start_event_loop(self, timeout=0):
    # docstring inherited
    # Set up a SIGINT handler to allow terminating a plot via CTRL-C.
    with _allow_interrupt_macos():
    self._start_event_loop(timeout=timeout) # Forward to ObjC implementation.

So, I think this makes sense as well. I can confirm that I upgraded to 14.3 and this fixes it. Thanks @ksunden!

@greglucas greglucas merged commit b50843d into matplotlib:main Feb 8, 2024
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Feb 8, 2024
@greglucas @meeseeksmachine
Backport PR matplotlib#27755: Allow threads during macos event loop
6ad534a
@meeseeksmachine meeseeksmachine mentioned this pull request Feb 8, 2024
Merged
QuLogic added a commit that referenced this pull request Feb 8, 2024
@QuLogic
Merge pull request #27759 from meeseeksmachine/auto-backport-of-pr-27…
4591c5b 
…755-on-v3.8.x

Backport PR #27755 on branch v3.8.x (Allow threads during macos event loop)
@tacaswell tacaswell mentioned this pull request Feb 8, 2024
Closed
@QuLogic
Copy link
Member

QuLogic commented Feb 14, 2024

@meeseeksdev backport to v3.7.x

@lumberbot-app Lumberbot (App)
Copy link

lumberbot-app bot commented Feb 14, 2024

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

  1. Checkout backport branch and update it.
git checkout v3.7.x
git pull
  1. Cherry pick the first parent branch of the this PR on top of the older branch:
git cherry-pick -x -m1 b50843d1ceac118ea5e71ef77fc591c8369954f7
  1. You will likely have some merge/cherry-pick conflict here, fix them and commit:
git commit -am 'Backport PR #27755: Allow threads during macos event loop'
  1. Push to a named branch:
git push YOURFORK v3.7.x:auto-backport-of-pr-27755-on-v3.7.x
  1. Create a PR against branch v3.7.x, I would have named this PR:

"Backport PR #27755 on branch v3.7.x (Allow threads during macos event loop)"

And apply the correct labels and milestones.

Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon!

Remember to remove the Still Needs Manual Backport label once the PR gets merged.

If these instructions are inaccurate, feel free to suggest an improvement.

QuLogic pushed a commit to QuLogic/matplotlib that referenced this pull request Feb 14, 2024
@greglucas @QuLogic
Merge pull request matplotlib#27755 from ksunden/release_gil_macos_ev…
d75d3a2 
…ent_loop

Allow threads during macos event loop

(cherry picked from commit b50843d)
@QuLogic QuLogic mentioned this pull request Feb 14, 2024
Merged
1 task
@QuLogic QuLogic modified the milestones: v3.8.3, v3.7.5 Feb 15, 2024
@ksunden ksunden mentioned this pull request Feb 21, 2024
Closed
@guyer guyer mentioned this pull request Feb 23, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tacaswell tacaswell tacaswell approved these changes

@greglucas greglucas greglucas approved these changes

Assignees
No one assigned
Labels
GUI: MacOSX
Projects
None yet
Milestone
v3.7.5
Development

Successfully merging this pull request may close these issues.

[Bug]: pyplot hangs at pause in sonoma 14.3 with backend MacOSX
4 participants
@ksunden @tacaswell @QuLogic @greglucas