Community
Participate
Working Groups
4.2 M3 If I have this code: new FileOutputStream(new File("C:\temp\foo.txt")).write(1); JDT does not report a resource leak. However if I simply add a local variable as follows: FileOutputStream out = new FileOutputStream(new File("C:\temp\foo.txt")); out.write(1); Now I get the resource leak reported. This is inconsistent, as the same leak exists in both cases.
I don't see an easy answer from the top of my head, but I'll take a look.
Stephan, I suppose this bug and bug 362332 means that we need to focus on the resource creation rather than resource variables. John, did you see this problem in 'real' code? If yes, how many instances of such a code pattern did you find? My guess would be that this code pattern does not occur too often in practice, but I could be wrong :)
(In reply to comment #2) > John, did you see this problem in 'real' code? If yes, how many instances of > such a code pattern did you find? My guess would be that this code pattern does > not occur too often in practice, but I could be wrong :) Since the pattern is not currently detected, I have no idea how common it is! In my case I didn't see it in 'real' code. I was in the process of creating a simplified sample case for bug 362332 when I noticed it.
(In reply to comment #2) > Stephan, I suppose this bug and bug 362332 means that we need to focus on the > resource creation rather than resource variables. "in addition to" not "rather than". All the flow analysis is based on local variables. If we want to analyze chained resources (see bug 360908) we need to investigate constructor invocations anyway. In that context I might be able to also address this issue.
Will be covered by the next patch in bug 358903.
The latest patch from bug 358903 comment 30 includes a solution for this bug. We will probably need a new set of error messages, because messages like 887 = Resource leak: ''{0}'' is never closed are meant to insert a variable name. However, the point in this bug is to report about values that are never assigned to any variable. I currently massaged this into the existing messages by saying: Resource leak: '<unassigned Closeable value>' is never closed Suggestions for better wording are welcomed :) Technical details: (In reply to comment #4) > (In reply to comment #2) > > Stephan, I suppose this bug and bug 362332 means that we need to focus on the > > resource creation rather than resource variables. > > "in addition to" not "rather than". The main analysis is still normal flow analysis for local variables. In addition, every AllocationExpression of an AutoCloseable type is investigated for: - is it a wrapper / resource-free? - are allocation arguments chained resources? These items are necessary for bug 358903. Normally, the results are then collected when analyzing the surrounding assignment (or local variable declaration), which then associates the allocated value to a local variable and we're back on our normal tracks. Specifically for this bug, an AllocationExpression can now also hold a FakedTrackingVariable so that applications of this value can detect that it is a closeable and at the end of the scope we can detect that such a value has never seen a close() call. While previously each FakedTrackingVariable was associated to a real variable (originalBinding), we now have tracking variables with no relation to any real variable (not a problem, just requires extra care). In the mentioned patch, tests for this bug can be found in the test062x group of tests.
Bug 358903 has resolved the better part of this issue, still when checking the outcome of ResourceLeakTests#test062c() I noticed that the analysis was too shy when a unassigned resource is passed as an argument to another method. This final issue has been resolved in commit e113fe138f1e4be2f22f54aa73cae2174e2d64a6 I'm marking this issue as closed. Still if anybody has a suggestion regarding the message (see comment 6) feel free to re-open.
If I use this class MyFile extends FileOutputStream { public MyFile(File file) throws FileNotFoundException { super(file); // TODO Auto-generated constructor stub } public MyFile write(int b, int a) throws IOException { // TODO Auto-generated method stub return this; } void foo() { new MyFile(new File("")).write(1, 1).close();} } I still see the resource leak warning even though close() has been called. I don't know if its easy to implement a check for this though. I leave it upto you to see if you want to look at it.
Verified for 3.8M5 using build I20120122-2000.
(In reply to comment #8) > If I use this > class MyFile extends FileOutputStream { > public MyFile(File file) throws FileNotFoundException { > super(file); > // TODO Auto-generated constructor stub > } > public MyFile write(int b, int a) throws IOException { > // TODO Auto-generated method stub > return this; > } > void foo() { new MyFile(new File("")).write(1, 1).close();} > } > > I still see the resource leak warning even though close() has been called. I > don't know if its easy to implement a check for this though. I leave it upto > you to see if you want to look at it. I humbly decline. The fix for this bug is based on the support to associate a FakedTrackingVariable to an AllocationExpression in order to investigate the directly enclosing syntactic context. However this expression: new MyFile(new File("")).write(1, 1) is beyond the field of vision of the analysis. How would we know that this expression represents the same instance as the AllocationExpression? I do not intend to implement a complete theorem prover :)
