Community
Participate
Working Groups
The following code generates a warning at the first line of the try block: static void saveDetails(byte[] detailsData) { OutputStream os = null; try { os = sContext.openFileOutput(DETAILS_FILE_NAME, Context.MODE_PRIVATE); os.write(detailsData); } catch (IOException e) { Log.w(LOG_TAG, "Unable to save details", e); } finally { if (os != null) { try { os.close(); } catch (IOException ignored) { } } } } The method openFileOutput is declared to throw FileNotFoundException. Oddly, similar code in other files does not generate the warning. The exact wording is: Potential resource leak: 'os' may not be closed
Created attachment 219521 [details] Snippet Having the attached snippet in workspace I see no warnings with default settings. Tod, could you check if I haven't twisted your example. Could you also try opening the same piece of code in a fresh workspace (to make sure you're running with default prefs).
(In reply to comment #1) > Having the attached snippet in workspace I see no warnings with default > settings. Tod, could you check if I haven't twisted your example. Could you > also try opening the same piece of code in a fresh workspace (to make sure > you're running with default prefs). Potential resource leaks are ignored by default. That should explain this difference, right? From looking at the code I'd say this is a duplicate of bug 385415. See there for background and workaround. I will re-open if that guess is wrong. *** This bug has been marked as a duplicate of bug 385415 ***
> Potential resource leaks are ignored by default. That should explain this > difference, right? Correct. I turned on most warnings, so this would not show up in a fresh workspace with default settings. > From looking at the code I'd say this is a duplicate of bug 385415. See > there for background and workaround. At first I didn't think it was a duplicate, because bug 385415 does not involve a finally clause, either in the bug or the proposed workarounds. However, it turns out that the warning goes away if I eliminate the try/catch surrounding the call to os.close(), which is kind of what I get out of bug 385415 after reading it over a couple of times. If that's how 385415 is understood, then yes, this is a duplicate. Would it be of value to add this case as a comment to that bug? It seems to be a slightly different code pattern than anything there. Or is having it here enough?
(In reply to comment #3) > Would it be of value to add this case as a comment to that bug? It seems to > be a slightly different code pattern than anything there. Or is having it > here enough? Adding this example as a test is a good idea, but having it in this bug is good enough. Thanks.
Bad news: work on bug 385415 shows that this is *not* a duplicate. This bug can be described as a result of out-of-order analysis: - the analysis first sees "os.close()" and assigns a resource tracker to os (using this tracker the resource is marked as closed) - later it sees "os = sContext.openFileOutput(..)" handling this as re-assigning a new resource to the same variable, creating a new tracker which is marked open. I.e., analysis "believes" we have two resources, of which only one is closed This is due to the internal ordering inside TryStatement.analyseCode(..), which is an intricate issue (designed long before I joined the team). As a result this bug must be investigated anew.
Test & fix have been released for 4.3 M2 via commit 0579ec46f093e76ebfb96f54aeedde906bdc6daa. In bug 388996 I fixed the general problem of out-of-order analysis as described in comment 5. For this bug I only had to improve the detection of that pattern: the analysis location can be (deeply) nested within a finally block -> iterate parents until null or FinallyFlowContext.
Verified for 4.3 M2 using Build id: I20120917-0800
