Find better way to stop the Python Notifier Thread

[pentschev]

In 22d42d6 we made sure all tests will properly stop the notifier thread. We would optimally want to make sure this occurs automatically, but we need to find a way that the ApplicationContext can stop the thread at some point, it is not clear how we would identify that the application no longer needs the thread running, but maybe weakrefs could help in that task.

[pentschev]

Further discussion:

@wence- :

I guess that the problem is that you don't want manual calling of ucp.finalize() or similar? You could do the cleanup in an atexit handler? If the thread lifetime is bound to the ApplicationContext you might be able to clean up the thread by using weakref.finalize

@pentschev :

I guess that the problem is that you don't want manual calling of ucp.finalize() or similar?

Yes, that's right. Ideally the lifetime of objects would take care of that, forcing the user to manually shutdown is not a good approach.

You could do the cleanup in an atexit handler? If the thread lifetime is bound to the ApplicationContext you might be able to clean up the thread by using weakref.finalize

atexit will only execute when the Python process is shutting down, right? If that's the case then it still wouldn't shutdown when context does, for example during pytests because the main process doesn't get destroyed until all tests complete.

@wence- :

Perhaps just

diff --git a/python/ucxx/_lib_async/application_context.py b/python/ucxx/_lib_async/application_context.py
index 6c624a0..fba5158 100644
--- a/python/ucxx/_lib_async/application_context.py
+++ b/python/ucxx/_lib_async/application_context.py
@@ -60,6 +60,7 @@ class ApplicationContext:
         self.start_notifier_thread()
 
         weakref.finalize(self, self.progress_tasks.clear)
+        weakref.finalize(self, self.stop_notifier_thread)
 
         # Ensure progress even before Endpoints get created, for example to
         # receive messages directly on a worker after a remote endpoint

?

@pentschev :

The problem is this will never occur, because self.notifier_thread will be always alive unless stop_notifier_thread() is called and thus will keep ApplicationContext alive as well. I attempted to make all references to the notifier thread weakrefs but failed to do so, it's possible I overlooked something in that case.

Anyway, after a conversation with Mads last week, I came up with a way to get rid of the notifier thread and move its object to the worker progress thread, which will greatly simplify things and make this piece go away if it succeeds, so we could wait a few more days before spending too much time on stop_notifier_thread(). So far I've managed to get it to work but it's really slow, so now I have to figure out what causes it to be slow and if it's possible to fix it.

@wence- :

Makes sense, thanks.