-
-
Notifications
You must be signed in to change notification settings - Fork 8k
macosx: Clean up single-shot timers correctly #27875
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works locally for me.
Are we able to clean up some of the timer handling on the Python side as well?
matplotlib/lib/matplotlib/backends/backend_macosx.py
Lines 70 to 72 in 0da2da0
This does appear to have resolved the segfault/obj-c level exception I was seeing. However, the test is still flaky for me locally (~half the time it fails as TimeoutExpired on the test subprocess, so not convinced we solved both problems...
This does appear to have resolved the segfault/obj-c level exception I was seeing. However, the test is still flaky for me locally (~half the time it fails as
TimeoutExpiredon the test subprocess, so not convinced we solved both problems...
Ah, that's unfortunate; I had hoped the timeout was due to having to create the coredump or something similar.
Are we able to clean up some of the timer handling on the Python side as well?
I think we can drop the stop in the callback. I'm not sure about the whole FigureCanvasMac._timers set, though. Do we need that to hold the strong reference on the Timer (Python) object?
The `NSTimer` docs state that a non-repeating (aka single-shot in our terms) timer is invalidated after it fires. This means that we should not do it ourselves, and in fact it appears that the pointer itself is no longer valid, so we would be passing an `invalidate` message to a random object or segfault.
60c1d5b to
9eda24f
Compare
Do we need that to hold the strong reference on the Timer (Python) object?
Yes, you are correct we need that reference somewhere.
However, the test is still flaky for me locally (~half the time it fails as TimeoutExpired on the test subprocess, so not convinced we solved both problems...
The test fails if I don't touch my mouse at all, but succeeds if the mouse enters the canvas area. Might have to do with the event loop not running when the canvas is in the background and needs to be woken up somehow from the signals coming in.
We are going to do a 3.8.4 due to numpy 2.0 churn so might as well fix a segfault/objective C error risk.
Might have to do with the event loop not running when the canvas is in the background and needs to be woken up somehow from the signals coming in.
Less than great for an automated test....
The test fails if I don't touch my mouse at all, but succeeds if the mouse enters the canvas area. Might have to do with the event loop not running when the canvas is in the background and needs to be woken up somehow from the signals coming in.
I don't think it is quite this clean in my experience... certainly something to it, but I have seen Xpasses happen without moving my mouse and failures when I have moved my mouse into the canvas, though it does seem to shift probabilities around...
...875-on-v3.8.x Backport PR #27875 on branch v3.8.x (macosx: Clean up single-shot timers correctly)
Uh oh!
There was an error while loading. Please reload this page.
PR summary
The
NSTimerdocs state that a non-repeating (aka single-shot in our terms) timer is invalidated after if fires. This means that we should not do it ourselves, and in fact it appears that the pointer itself is no longer valid, so we would be passing aninvalidatemessage to a random object or segfault.(削除) I believe this is whytest_other_signal_before_sigintis marked as flaky as well, as it uses a single-shot timer and a segfault is recorded in the XPASS log. (削除ここまで)PR checklist