Skip to content

Don't associate Wx timers with the parent frame. #11590

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

Merged
merged 1 commit into from
Jul 9, 2018

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Jul 6, 2018

This is consistent with the behavior on Qt and GTK, and avoids a
segfault due to lack of disconnection of the timer after the parent
widget is destroyed (otherwise, we'd need to keep track of timers
associated with each widget and tear them down when the widget is
destroyed).

Closes #11582.

Marking as release-critical per "fixes a segfault", though not a regression so not insisting on it.

PR Summary

PR Checklist

  • Has Pytest style unit tests
  • Code is PEP 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

@anntzer anntzer added Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions. GUI: wx labels Jul 6, 2018
@anntzer anntzer added this to the v3.0 milestone Jul 6, 2018
@tacaswell tacaswell modified the milestones: v3.0, v2.2.3 Jul 7, 2018
@tacaswell
Copy link
Member

This looks like it also fixes #11578 for wxagg and changes the failure on wx to a timeout.

Can you rebase this on master and re-enable the wxagg tests?

@anntzer
Copy link
Contributor Author

anntzer commented Jul 8, 2018

done

@anntzer anntzer force-pushed the parentlesswxtimers branch from 03ed370 to 9d4cc11 Compare July 8, 2018 15:58
@anntzer
Copy link
Contributor Author

anntzer commented Jul 8, 2018

Doesn't segfault travis anymore, but still fails (as you noted, on a timeout). Let's keep this wx test disabled?

@tacaswell
Copy link
Member

Yes, leave wx disabled, but reanable wxagg. wx is deprecated aynway.

@anntzer
Copy link
Contributor Author

anntzer commented Jul 8, 2018

I'm puzzled why wx fails when wxagg works (they should share the same code re: interactivity), but sure...

@anntzer anntzer force-pushed the parentlesswxtimers branch from 9d4cc11 to 27e9c2b Compare July 8, 2018 19:00
@tacaswell
Copy link
Member

Given that it is timing out probably something subtle in the draw / render / put the window on the (fake) screen process?

@anntzer
Copy link
Contributor Author

anntzer commented Jul 8, 2018

Well, that still fails. Let's just get rid of the wx test.

This is consistent with the behavior on Qt and GTK, and avoids a
segfault due to lack of disconnection of the timer after the parent
widget is destroyed (otherwise, we'd need to keep track of timers
associated with each widget and tear them down when the widget is
destroyed).
@tacaswell
Copy link
Member

@DietmarSchwertberger Can you review this?

@tacaswell tacaswell modified the milestones: v2.2.3, v3.0 Jul 9, 2018
@DietmarSchwertberger DietmarSchwertberger merged commit c4aebac into matplotlib:master Jul 9, 2018
@DietmarSchwertberger
Copy link
Contributor

Looks good to me. Pending events are the most common source of segfaults...

@tacaswell
Copy link
Member

Thanks @DietmarSchwertberger !

@anntzer anntzer deleted the parentlesswxtimers branch July 10, 2018 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GUI: wx Release critical For bugs that make the library unusable (segfaults, incorrect plots, etc) and major regressions.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

wx segfault
3 participants