Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 473015

Summary: [compiler][resource] Resource leak error when copying/wrapping closable variable
Product: [Eclipse Project] JDT Reporter: Mark Jeronimus <mark.jeronimus>
Component: CoreAssignee: Stephan Herrmann <stephan.herrmann>
Status: CLOSED WONTFIX QA Contact:
Severity: minor    
Priority: P4 CC: stephan.herrmann
Version: 4.5   
Target Milestone: ---   
Hardware: PC   
OS: Windows Server 2003   
Whiteboard: stalebug

Description Mark Jeronimus CLA 2015-07-19 06:19:23 EDT
try (InputStream in = connection.getInputStream()) {
	InputStream in2 = in; // Resource leak: 'in2' is never closed
}

This error is unjustified because whenever 'in' is closed, 'in2' is closed also.

A simple workaround, although ridiculous because of the aforementioned reason:

try (InputStream in = connection.getInputStream()) {
	try (InputStream in2 = in) {
	}
}

The workaround doesn't work in the following case (which is my production code):

try (InputStream in = connection.getInputStream()) {
	InputStream in2 = in; //
	if (gzipped) {
		in2 = new GZIPInputStream(in2);
	}
}

This is also unjustified because whenever 'in' is closed, using 'in2' would cause it to throw IOExceptions. But in2 can't be used after 'in' is closed because 'in2' is local within the try-with-resource.

I could put this check in a ternary inside the try-with-resource header, but it gets really messy when there are more ways to conditionally wrap the stream.
Comment 1 Stephan Herrmann CLA 2015-07-19 07:08:54 EDT
I see. 

A full solution to this issue would require a complete points-to analysis of all references. Such analysis is beyond the capabilities of the compiler.

First, I recommend to avoid assigning managed resources to secondary local variables, if possible.

I still see a little room for improvement, when comparing the direct assignment with the one wrapping in in a GZIPInputStream.
This is OK:
//---
    try (InputStream in = connection.getInputStream()) {
        InputStream in2;
        if (gzipped) {
            in2 = new GZIPInputStream(in);
        } else {
            in2 = new GZIPInputStream(in);
        }
        //...
    }
//---

while this raises the warning:
//---
    try (InputStream in = connection.getInputStream()) {
        InputStream in2;
        if (gzipped) {
            in2 = new GZIPInputStream(in);
        } else {
            in2 = in;
        }
    }
//---

Perhaps we can use some of the smarts applied to resource wrapping also to the plain assignment. This would, however, be low priority for me.

> I could put this check in a ternary inside the try-with-resource header, but 
> it gets really messy when there are more ways to conditionally wrap the stream.

Workaround: extract the code that determines the resource into a helper method.
Comment 2 Eclipse Genie CLA 2020-03-15 17:01:12 EDT
This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. As such, we're closing this bug.

If you have further information on the current state of the bug, please add it and reopen this bug. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

--
The automated Eclipse Genie.