Bug 365710 - FUP of bug 363293: Fix the incorrect added resource close
Summary: FUP of bug 363293: Fix the incorrect added resource close
Status: VERIFIED FIXED
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.8   Edit
Hardware: PC Windows 7
: P3 minor (vote)
Target Milestone: 3.8 M7   Edit
Assignee: Stephan Herrmann CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on: 362332 368546
Blocks:
  Show dependency tree
 
Reported: 2011-12-06 06:43 EST by Satyam Kandula CLA
Modified: 2012-04-30 08:48 EDT (History)
3 users (show)

See Also:

Attachments
Proposed patch (747 bytes, patch)
2011-12-06 06:44 EST, Satyam Kandula CLA 		no flags Details | Diff
slightly more explicit fix (1.11 KB, patch)
2012-04-24 06:02 EDT, Stephan Herrmann CLA 		no flags Details | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Satyam Kandula CLA 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 CLA 2011-12-06 06:44:04 EST 
Created attachment 207972 [details]
Proposed patch
Comment 2 Stephan Herrmann CLA 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 CLA 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 CLA 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 CLA 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 CLA 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 CLA 2012-04-24 06:32:37 EDT 
(In reply to comment #6)
Please go ahead and push it.
Comment 8 Stephan Herrmann CLA 2012-04-24 09:59:15 EDT 
Released for 3.8 M7 via commit ebee4ac330d3dc7dc9f8f11cab338cf905bf6dd5
Comment 9 Satyam Kandula CLA 2012-04-30 08:48:14 EDT 
Verified for 3.8M7 by looking at the code.