Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
tacaswell merged 1 commit intomatplotlib:mainfromQuLogic:fix-macosx-timer
Mar 14, 2024

Conversation

QuLogic
Copy link
Member

@QuLogicQuLogic commentedMar 7, 2024
edited
Loading

PR summary

TheNSTimer docs 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 aninvalidate message to a random object or segfault.

I believe this is whytest_other_signal_before_sigint is marked as flaky as well, as it uses a single-shot timer and a segfault is recorded in the XPASS log.

PR checklist

Copy link
Contributor

@greglucasgreglucas left a 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?

# We need to explicitly stop and remove the timer after
# firing, otherwise segfaults will occur when trying to deallocate
# the singleshot timers.

@ksunden
Copy link
Member

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 asTimeoutExpired on the test subprocess, so not convinced we solved both problems...

@QuLogic
Copy link
MemberAuthor

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 asTimeoutExpired on 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.

@QuLogic
Copy link
MemberAuthor

Are we able to clean up some of the timer handling on the Python side as well?

I think we can drop thestop in the callback. I'm not sure about the wholeFigureCanvasMac._timers set, though. Do we need that to hold the strong reference on theTimer (Python) object?

The `NSTimer` docs state that a non-repeating (aka single-shot in ourterms) timer is invalidated after it fires. This means that we shouldnot do it ourselves, and in fact it appears that the pointer itself isno longer valid, so we would be passing an `invalidate` message to arandom object or segfault.
@greglucas
Copy link
Contributor

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.

@tacaswelltacaswell modified the milestones:v3.9.0,v3.8.4Mar 8, 2024
@tacaswell
Copy link
Member

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.

@tacaswell
Copy link
Member

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....

@ksunden
Copy link
Member

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 itdoes seem to shift probabilities around...

@QuLogicQuLogic deleted the fix-macosx-timer branchMarch 14, 2024 19:48
QuLogic added a commit that referenced this pull requestMar 14, 2024
…875-on-v3.8.xBackport PR#27875 on branch v3.8.x (macosx: Clean up single-shot timers correctly)
@ksundenksunden mentioned this pull requestApr 4, 2024
5 tasks
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers

@greglucasgreglucasgreglucas approved these changes

Assignees
No one assigned
Projects
None yet
Milestone
v3.8.4
Development

Successfully merging this pull request may close these issues.

4 participants
@QuLogic@ksunden@greglucas@tacaswell

[8]ページ先頭

©2009-2025 Movatter.jp