jetty.project
jetty.project copied to clipboard
Fix buffer leaks in FCGI and H3 `HttpClientIdleTimeoutTest`
testClientIdleTimeout and testRequestIdleTimeout leak a buffer because the handler's callback is never completed and they don't give the server a chance to reclaim its resources after idle timeout.
Note: the leak was only detected in FCGI because the client was not uploading any content. Adding some uploaded content uncovered an equivalent leak in H3.
See https://github.com/eclipse/jetty.project/issues/10226
So this leak was entirely contained within the test case. Interesting. Wonder if we should document how to turn on Leak tracking in our developer guide?
I'm open to the idea, but the leak tracking buffer pool currently is rough on the edges, so I'm not keen on exposing it to end users as it currently is. @gregw had a similar suggestion here (https://github.com/eclipse/jetty.project/pull/10325#issuecomment-1695235810), and we agreed that, as it stands, the leak tracker has little value outside the scope of our tests.
This requires more investigation as other protocols manage to release their buffers despite the callback not being completed. We, at the very least, need to understand why not FCGI.
On hold until https://github.com/eclipse/jetty.project/issues/10277 is sorted.
@sbordet @gregw @joakime I've reworked these tests by using Request.addFailureListener() to detect when the server had a chance to reclaim its resources, identified and fixed the leak in FCGI and slightly improved the tests such as a similar leak became apparent in H3 which I also fixed.
This PR is finally ready for review.
@lorban the OSGI CI failures look like hard failures. @janbartel any ideas to help out?
@gregw @lorban there is a new osgi test that has been added which is failing in this branch. It seems like the new test class is there, but the new xml file that should be present in the jar is not there? Could this be a build cache issue? @olamy ?
I can't see any more issues after rebasing from jetty-12 branch
@sbordet nudge