Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 351400 - [1.7][compiler] Warn for multiple close of an AutoCloseable resource
Summary: [1.7][compiler] Warn for multiple close of an AutoCloseable resource
Status: NEW
Alias: None
Product: JDT
Classification: Eclipse Project
Component: Core (show other bugs)
Version: 3.7   Edit
Hardware: All All
: P5 enhancement (vote)
Target Milestone: ---   Edit
Assignee: JDT Core Triaged CLA
QA Contact: Ayushman Jain CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2011-07-07 04:27 EDT by Ayushman Jain CLA
Modified: 2012-01-28 20:13 EST (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ayushman Jain CLA 2011-07-07 04:27:10 EDT
BETA_JAVA7

The spec for AutoCloseable.close() says:

* <p>Note that unlike the {@link java.io.Closeable#close close}
     * method of {@link java.io.Closeable}, this {@code close} method
     * is not required to be idempotent.  In other words,
     * calling this {@code close} method more than once may have some
     * visible side effect, unlike {@code Closeable.close} which is
     * required to have no effect if called more than once.


Hence when a user is explicitly closing a resource of type AutoCloseable, or closing it more than once in a try with resources block, we can give a warning to the user.

class X{
   void foo() throws Exception {
       try (FileInputStream fis = new FileInputStream) {
                fis.close();          // new warning here
       } finally {
       }
   }
}
Comment 1 Deepak Azad CLA 2011-07-07 05:05:25 EDT
The spec also says

 * However, implementers of this interface are strongly encouraged
 * to make their {@code close} methods idempotent.
 
So, I would say that this warning is a bit of overkill.
Comment 2 Markus Keller CLA 2011-07-07 12:43:38 EDT
(In reply to comment #1)
+1, wouldn't do that.
Comment 3 Stephan Herrmann CLA 2011-07-07 14:45:56 EDT
If close() is indeed idempotent an explicit close within t-w-r is not
dangerous but simply unnecessary code.

Given the infra structure from bug 349326 implementing such warning would
be close to trivial. So I guess the most relevant overhead would be the
configurability (CompilerOptions, JavaCore, @SuppressWarnings, UI)?

Just let me know if there's still interest in this warning.
Comment 4 Ayushman Jain CLA 2011-07-08 09:20:54 EDT
(In reply to comment #3)
> Given the infra structure from bug 349326 implementing such warning would
> be close to trivial. So I guess the most relevant overhead would be the
> configurability (CompilerOptions, JavaCore, @SuppressWarnings, UI)?
> 
> Just let me know if there's still interest in this warning.

Yes currently I don't feel very strongly about having this, but just filed a bug to gather other's opinions. Surely, if the infrastructure from bug 349326 makes it simple, we can think about having this for 3.8 or later
Comment 5 Olivier Thomann CLA 2011-09-30 11:23:33 EDT
Can we reassess this feature based on the fix made for bug 349326 ?
Comment 6 Deepak Azad CLA 2011-09-30 12:15:50 EDT
I don't think the warning itself is too interesting to be shown in the UI. However, if the warning is not too hard to implement JDT UI could use the warning internally in a clean up to get rid of unnecessary close() invocations.
Comment 7 Srikanth Sankaran CLA 2012-01-28 20:13:37 EST
I think this should be closed as WONTFIX. The comments make for slightly
confusing reading: comment# 0 javadoc citation is actually a case for 
NOT issuing the warning, but it requests it. comment#1 citation actually
strengthens the case for the warning, but the end argument is against it.