Simplified URLResource cleaner #10061
Conversation
The reference to the InputStream does not need to be soft/weak as a hard reference to it does not prevent the URLResource itself from being collected. Also by making a runnable record, there is one less allocation per input stream creation.
| if (ON_SWEEP_LISTENER != null) | ||
| ON_SWEEP_LISTENER.accept(ref); | ||
| }); | ||
| CLEANER.register(this, new InputStreamCleaner(in)); |
There was a problem hiding this comment.
Rereading this code, I noticed the input stream gets closed if the URLResource gets garbage collected.
This means we get this:
Resource webResource = resource.resolve("/web.xml");
assertTrue(webResource.exists());
InputStream inputStream = webResource.newInputStream();
webResource = null;
await().atMost(5, TimeUnit.SECONDS).until(() ->
{
System.gc();
return cleanedRefCount.get() > 0;
});
assertThat(IO.toString(inputStream), notNullValue()); // this throws IOException: Stream closedThere was a problem hiding this comment.
Hmmm perhaps we need to weak reference the input stream itself?
But it will generally be fine as I don't the reference to the input stream outlives the resource reference often.
Either way, this is better than what is committed.
|
@lorban thanks for the green tick... But I think I have an idea to simply solve that problem... I'll try tonight.... |
Use AtomicReference for the inputstream, so that if it is taken, then it is nulled and will not be closed.
|
@lorban what about now? |
|
@lorban I think the test is a bit flakey. sometimes it does not complete when I run it from the IDE |
Improved tests
|
@lorban I fixed the tests and now test for the inputstream not being closed if it is taken |
The reference to the InputStream does not need to be soft/weak as a hard reference to it does not prevent the URLResource itself from being collected. Also by making a runnable record, there is one less allocation per input stream creation.