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

Bug 361073

Summary:	Avoid resource leak warning when the top level resource is closed explicitly
Product:	[Eclipse Project] JDT	Reporter:	Deepak Azad <deepakazad>
Component:	Core	Assignee:	Stephan Herrmann <stephan.herrmann>
Status:	VERIFIED FIXED	QA Contact:
Severity:	normal
Priority:	P3	CC:	amj87.iitr, gary_shank, markus.kell.r
Version:	3.8
Target Milestone:	3.8 M5
Hardware:	PC
OS:	Windows 7
Whiteboard:
Bug Depends on:	358903
Bug Blocks:

Description Deepak Azad

2011-10-15 23:34:23 EDT

Opposite of bug 360908. 

Here 'reader' is closed but there is a potential resource leak warning for the underlying 'stream'.
-------------------------------------------------------------------------------
boolean loadURL(final URL url) throws IOException {
	InputStream stream = null;
	BufferedReader reader = null;
	try {
		stream = url.openStream();
		reader = new BufferedReader(new InputStreamReader(stream));

		boolean doRead = true;
		while (doRead) {
			System.out.println(reader.readLine());
		}
	} finally {
		try {
			if (reader != null)
				reader.close();
		} catch (IOException x) {
		}
	}
	return false; // 'stream' may not be closed at this location
}
-------------------------------------------------------------------------------

Actual example from o.e.jdt.ui
Potential resource leak: 'contents' may not be closed at this location	DeletePackageFragmentRootChange.java	/org.eclipse.jdt.ui/core refactoring/org/eclipse/jdt/internal/corext/refactoring/changes	line 171

Comment 1 Stephan Herrmann

2011-12-06 09:10:26 EST

I'm not 100% sure how we should read this.

After creating the chaining readers, should we assume all responsibility
lies in closing the outer reader? In that case we would miss if the
stream is closed but not the reader.

It seems we need to record the logical dependency between both,
which would make the implementation a bit more complex.
Therefore I'm not making a promise for M5 at this point.

Comment 2 Deepak Azad

2012-01-06 11:24:11 EST

Without patch v0.6 from bug 358903
- There is a warning on the return statement

With patch v0.6 from bug 358903
- The warning moves to declaration of 'in'
- Extract "new InputStreamReader(in)" to a local variable and the warning moves back to the return statement.
=> The warning should be completely eliminated. No ?
-------------------------------------------------------------------
	String[] bar() throws Exception {
		InputStream in = new FileInputStream(new File(""));
		BufferedReader reader = new BufferedReader(new InputStreamReader(in),
				512);
		List<String> fileContentStore = new ArrayList<>();
		try {
			String line;
			while ((line = reader.readLine()) != null) {
				fileContentStore.add(line);
			}
			return fileContentStore
					.toArray(new String[fileContentStore.size()]);
		} finally {
			reader.close();
		}
	}
-------------------------------------------------------------------

Comment 3 Deepak Azad

2012-01-06 11:25:53 EST

(In reply to comment #2)
That was a real test case:
Resource leak: 'in' is never closed	SyncFileWriter.java	/org.eclipse.team.cvs.core/src/org/eclipse/team/internal/ccvs/core/util	line 495

Comment 4 Stephan Herrmann

2012-01-07 20:21:49 EST

Covered by the patch in bug 358903 comment 20.
Corresponding tests are: test061e, test061f.

Comment 5 Stephan Herrmann

2012-01-15 09:29:23 EST

Resolved by commit 8d45cb26fc5ad244f93e8632d761d46ad4a120cf
on behalf of bug 358903.

Comment 6 Ayushman Jain

2012-01-24 09:54:06 EST

Verified for 3.8M5 using build I20120122-2000

Comment 7 Gary Shank

2012-08-04 09:00:36 EDT

I just downloaded/upgraded my Indigo (3.7) eclipse to Juno (3.8) this morning (Aug 4th, 2012) and now I'm getting the "resource leak" issue in my code if I throw an exception like this:

public void test() {
    BufferedReader br = null;
    try {
        br = new BufferedReader(new FileReader("blah"));
        String line = null;
        while ( (line = br.readLine()) != null ) {
            if ( line.startsWith("error") )
                throw new Exception("error"); //Resource leak: 'br' is not closed at this location
        }
    } catch (Throwable t) {
        t.printStackTrace();
    } finally {
        if ( br != null ) {
            try { br.close(); }
            catch (Throwable e) { br = null; }
        }
    }
}

Is this somehow different from this bug that is supposed to be fixed?  Am I missing something maybe not downloaded/upgraded when I went from Indigo to Juno?  Do I need to open a new bug?
Thanks,
Gary

Comment 8 Stephan Herrmann

2012-08-04 10:22:04 EDT

(In reply to comment #7)

Thanks for reporting. 
I made a comment in bug 385415 to check this example before releasing the fix.
No need to file a new bug at this point.