Skip to content

Conversation

@mkundu1
Copy link
Contributor

@mkundu1 mkundu1 commented Sep 22, 2023

session.exit() is handled in a daemon thread (MonitorThread) which ensures shutdown of all streaming (non-daemon) threads. A daemon thread is terminated abruptly during Python process exit, after all non-daemon threads are exited). In the current code, the streaming threads are shut down after the server is exited within session.exit(). The MonitorThread shuts down immediately after shutting down the streaming threads without exiting the last server.

In this PR, a long-running thread is added which is exited at the end of session.exit() to ensure everything within session.exit() gets executed during exit.

Need to see how to unittest this (need a Python process exit).

Copy link
Member

@raph-luc raph-luc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, and nice that you are adding a test that can work both with and without containers as well

Very minor suggestion would be to rename _exit_evt to _exit_event just to make this var name clearer/obvious

@mkundu1
Copy link
Contributor Author

mkundu1 commented Sep 25, 2023

LGTM, and nice that you are adding a test that can work both with and without containers as well

Very minor suggestion would be to rename _exit_evt to _exit_event just to make this var name clearer/obvious

Thanks for the suggestions, they are very helpful.

@mkundu1 mkundu1 merged commit af6361e into main Sep 25, 2023
@mkundu1 mkundu1 deleted the fix/exit branch September 25, 2023 15:12
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

Successfully merging this pull request may close these issues.

Not all Fluent session exit on Python exit if watchdog is not used

6 participants