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

Bug 365710

Summary:

FUP of bug 363293: Fix the incorrect added resource close

Product:

[Eclipse Project] JDT

Reporter:

Satyam Kandula <satyam.kandula>

Component:

Core

Assignee:

Stephan Herrmann <stephan.herrmann>

Status:

VERIFIED FIXED

QA Contact:

Severity:

minor

Priority:

CC:

satyam.kandula, srikanth_sankaran, stephan.herrmann

Version:

3.8

Target Milestone:

3.8 M7

Hardware:

OS:

Windows 7

Whiteboard:

Bug Depends on:

362332, 368546

Bug Blocks:

Attachments:

Description	Flags
Proposed patch	none
slightly more explicit fix	none

Description Satyam Kandula

2011-12-06 06:43:09 EST

For bug 363293, resource close methods have been added to some tests. However, the change in AbstractCompilerToolTest.java is not correct. 
With the change the file manager object gets closed even if it is passed as an argument. Though the current testcases doesn't get impacted, I think we shouldn't do this change.

Comment 1 Satyam Kandula

2011-12-06 06:44:04 EST

Created attachment 207972 [details]
Proposed patch

Comment 2 Stephan Herrmann

2011-12-06 08:20:48 EST

I'll use bug 362332 to change the reported warning into saying 
"potential leak".
After that I'll apply the test change.

Comment 3 Stephan Herrmann

2012-01-15 11:33:06 EST

I just checked whether 362332 indeed solved this issue, but unfortunately
we'll have to wait for the fix from bug 368546, too.

Relaxing target milestone from 3.8 M5 to 3.8.

Comment 4 Srikanth Sankaran

2012-03-20 11:32:59 EDT

If you plan to include a fix for this in 3.8 M7, please adjust the
target suitably, so it becomes easier to track.

Comment 5 Stephan Herrmann

2012-03-20 11:42:54 EDT

I assume this should be easy now, as both pre-reqs are resolved now.

Comment 6 Stephan Herrmann

2012-04-24 06:02:55 EDT

Created attachment 214450 [details]
slightly more explicit fix

OK, an unconditional close is no longer needed to silence the warning.

Satyam, do you want to review the patch (which basically does the same as what you already proposed) or is it OK to push without review since it's a tests-only patch?

Comment 7 Satyam Kandula

2012-04-24 06:32:37 EDT

(In reply to comment #6)
Please go ahead and push it.

Comment 8 Stephan Herrmann

2012-04-24 09:59:15 EDT

Released for 3.8 M7 via commit ebee4ac330d3dc7dc9f8f11cab338cf905bf6dd5

Comment 9 Satyam Kandula

2012-04-30 08:48:14 EDT

Verified for 3.8M7 by looking at the code.