Feature/closeables weak ref

[JerryShea]

WIP - do not merge

Fixes two problems:

1. makes sure that tailers and appenders remove themselves from SingleChronicleQueue.closers
2. makes closers a WeakReference so that the SCQ is no longer the only thing holding a reference to tailers & appenders after they would otherwise be candidates for GC

Second point above we should definitely discuss.

If we can reach consensus on this then after this change has been merged, it will be possible to set disable.discard.warning=false to register a finalizer for both appenders and tailers and ensure that if the finalizer is called, that they will warn and close themselves, thus releasing any resources. Backporting this to 23 will not be trivial as discard warnings/finalizers were different back then.

[JerryShea]

I reverted the first change (to call removeCloseListener from appender/tailer close methods) as a bunch of tests failed - will come back to.

The second change gives us some test flakiness with tests occasionally failed with exceptions were detected: Discarded without closing... and the tests that have shown this have tailers which are not explicitly closed and could go out of scope (and thus be GC'd) before the parent queue is closed. I have fixed a few of these tests to close tailers explicitly but there are a lot of them. We can a) fix all these tests - it would be the ideal situation for our tests to demonstrate proper lifecycle management OR b) ignoreException the "Discarded without closing" warning and create an issue to fix the tests

[sonarqubecloud[bot]]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
5 Code Smells

64.3% Coverage
0.0% Duplication

[nicktindall]

I reverted the first change (to call removeCloseListener from appender/tailer close methods) as a bunch of tests failed - will come back to.

The second change gives us some test flakiness with tests occasionally failed with exceptions were detected: Discarded without closing... and the tests that have shown this have tailers which are not explicitly closed and could go out of scope (and thus be GC'd) before the parent queue is closed. I have fixed a few of these tests to close tailers explicitly but there are a lot of them. We can a) fix all these tests - it would be the ideal situation for our tests to demonstrate proper lifecycle management OR b) ignoreException the "Discarded without closing" warning and create an issue to fix the tests

I vote definitely not b - unless it's done locally on the problematic tests, but if you're doing that you might as well fix them. "Creating an issue" no way guarantees it will ever get done, in the meantime, we are muting the thing we use to detect resource leaks so we will run the risk of introducing resource leaks. If it's not tested it's broken.

[sonarqubecloud[bot]]

Quality Gate failed

Failed conditions

16.67% Condition Coverage on New Code (required ≥ 35%)

See analysis details on SonarCloud