[bisq-network/bisq] Fix resource leak caused by not cleaning up unused threads (#3734)

Julian Knutsen notifications at github.com
Wed Dec 4 02:25:04 UTC 2019


julianknutsen approved this pull request.

ACK

Luckily verifying this didn't take the two days you suggested. I spent some time learning more about jprofiler views and updating the executor to set the thread name so it could be tracked. Here is the comparison before and after your patch to verify the threads no longer leak:

~20 minute uptime

**Before: (bundleSender > # connected peers)**
![9V12](https://user-images.githubusercontent.com/8082291/70106271-05b45b00-15f8-11ea-99d8-a3f04c6b25e5.JPG)


**After: (bundleSender == # connected peers)**
![8v8](https://user-images.githubusercontent.com/8082291/70106453-8a9f7480-15f8-11ea-9d79-5a833bbef670.JPG)

>From the heap inspection, it looks like each executor is about 300-600 bytes each so that is the magnitude of the leak on each disconnect that isn't cleaned up.

**Testing**

I ran this through a few localnet smoke tests that take nodes up and down cases to verify no egregious stack traces. As suggested, it is probably a good idea to rerun trade tests that take nodes up and down if this is cherry-picked to the branch.

I audited the code and this seems low risk. It follows the same pattern as the other executor in the Connection object so any bug in the shutdown path would likely have triggered there already.



-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/bisq-network/bisq/pull/3734#pullrequestreview-326568529
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.bisq.network/pipermail/bisq-github/attachments/20191203/0a0a613d/attachment.html>


More information about the bisq-github mailing list