fix: memory leak when publishing messages #406
Conversation
If publish threads are marked as daemonic, the leak seemingly disappears.
|
Server 500 when running samples, re-starting. |
plamut
left a comment
There was a problem hiding this comment.
Found a true cause of the issue, it's a bug in CPython.
The additional linked comment explains which `CPython` issue is the root cause of this.
|
The fix is good, but I wonder if we'd run into issues by setting these threads as daemons.
So, this would mean that scripts that may have created a publisher client, published a bunch of messages, and had only exited when the commit threads (non-daemon) finished running - these scripts could now terminate earlier. However, this may not be a problem, because if users want to guarantee publishes happened, they need to wait for the publish futures to finish with success. Anyway, just wondering what your thoughts were here. |
|
Peter and I spoke offline: possible premature script termination b/c the threads are no daemons doesn't break a customer guarantee, because as long as the user hasn't received a completed future, they can't assume the publish succeeded. |
|
Since it's already late into the workweek, I'll cut a release containing this on Monday. |
Fixes #395.
If publish threads are marked as daemonic, the leak seemingly disappears.
How to verify
Set up a topic and a subscription, install
memoryprofiler. Add the@profiledecorator to themain()function from the sample code and run it (with and without the fix):After each run, plot the memory usage graph:
The graphs should look similar to the ones posted here.
PR checklist