Skip to content
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

Use a weakref finalizer to stop notifier thread #143

Open
wants to merge 3 commits into
base: branch-0.36
Choose a base branch
from
Open

Use a weakref finalizer to stop notifier thread #143

wants to merge 3 commits into from

Commits on Dec 6, 2023

  1. Use a weakref finalizer to stop notifier thread 

    When creating our own application context with the python notifier
thread enabled we previously had to arrange to stop it before dropping
the reference to the context we created. The consequence of not doing
so is that our application hangs in exit waiting for a thread to
join that never will.

Applying the principle of least surprise, an object should clean up
all of its resources when dropping out of scope. To ensure this
change Application.stop_notifier_thread to a staticmethod and attach
it as a weakref finalizer when starting the notifier thread. This way,
we don't have to handle the case where there is no notifier thread,
nor do we have to remember to call the stop function manually.

This additionally allows us to clean up the top-level reset function
since there is no longer a need to stop the notifier "manually" during
reset.
    @wence-
    wence- committed Dec 6, 2023
    Configuration menu
    Copy the full SHA
    56c3ae7 View commit details
    Browse the repository at this point in the history

  2. Reinstate top-level stop_notifier_thread

    @wence-
    wence- committed Dec 6, 2023
    Configuration menu
    Copy the full SHA
    3ea3043 View commit details
    Browse the repository at this point in the history

  3. Squash, hopefully, a few more refcycles for cleanup

    @wence-
    wence- committed Dec 6, 2023
    Configuration menu
    Copy the full SHA
    e5170e7 View commit details
    Browse the repository at this point in the history