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.

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

Observer.unchedule_all not waiting for threads to end #252

Closed
adiroiban opened this issue Jul 16, 2014 · 4 comments
Closed

Observer.unchedule_all not waiting for threads to end #252

adiroiban opened this issue Jul 16, 2014 · 4 comments

Comments

@adiroiban
Copy link
Contributor

I assume that in most use cases the observer is started once, and then closes at process exit.

I want to write some integration automated test for my code which uses watched and at the end of each test, I check that no thread were left as side effect of running the test.

I ended up with this code

class ObserverThread(Observer):
    """
    Patched watched.Observer.
    """

    def _stopThread(self, thread):
        """
        Stop thread and wait for it to end.

        An exception is raised if thread was not completely stopped.
        """
        thread.stop()
        thread.join(1)
        if thread.isAlive():
            raise AssertionError('Failed to stop thread: %s' % thread)

    def _clear_emitters(self):
        for emitter in self._emitters:
            self._stopThread(emitter)

        self._emitters.clear()
        self._emitter_for_watch.clear()

If if think that this is useful I can send a patch

Cheers

@adiroiban
Copy link
Contributor Author

Thread.join() will block forever... I am not sure that this is the best approach. I prefer to raise a timeout error so that users can see what went wrong... otherwise they will just see the code being blocked.

... but this is a matter or taste... still, i think that it would be nice to have the thread close/join code in a separate method so that it would be easier (at least for users like me) to overwrite and reduce code duplication.

cheers

@tamland
Copy link
Collaborator

tamland commented Jul 17, 2014

Well, if join fails and you ignore it, it still deadlocks and the interpreter needs to kill it. The only reason it currently works without join is because all the threads in watchdog are daemon threads. It's an ugly hack to hide threading issues (like your #250) and should really be removed once threading is completely fixes.

Ps: 1 second is very little time to stop the emitters. For instance the poller won't stop in the middle of taking a snapshot. Worst case scenario a single snapshot can take like 10 seconds.

@tamland
Copy link
Collaborator

tamland commented Jul 17, 2014

I'm not quite sure why you would ever need to overload stopping of emitters. They are supposed to be handled internally by the Observer. There's also on_thread_stop for doing other stuff in both emitters and observers.

@adiroiban
Copy link
Contributor Author

True, my example with 1 second is not always practical..and is ok to have 10 second.

I would want to overwrite stopping of emitter to insert custom join timeout and join logic... so on_tread_stop will not help me.

I forgot to mention that I did removed the 'daemon' flag from all threads created by watchdog. I had no idea why that was there in the first place.

my application adds/removes observers at runtime, so I don't want to wait forever for join() and show an error message / log something when things go wrong.

CCP-Aporia pushed a commit to CCP-Aporia/watchdog that referenced this issue Aug 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants