The fix revision 907502 was aimed to remove an resource leak bug on the FileOutputStream object "fos" and the ObjectOutputStream "oos" in the method "doUnload" of the file "/tomcat/tc6.0.x/trunk/java/org/apache/catalina/session/StandardManager.java" , but it is incomplete. When the ObjectOutputStream object is created unsuccessfully but the temp BufferedOutputStream object is created successfully, the temp object will be leaked. Besides that, the "oos" is not closed in all the pathes. The best way to close such resource object is putting such close operations in the finaly block of a try-catch-finally structure.
Severity set to something more realistic given the likelihood of this ever actually happening. Fixed in trunk and 7.0.x for 7.0.27. Proposed for 6.0.x although I'd be equally happy with WONTFIX for 6.0.x.
> When the ObjectOutputStream object is created unsuccessfully but the temp BufferedOutputStream object is created successfully, the temp object will be leaked. BufferedOutputStream does not hold any native resources. Its close() method is mostly a noop. There is no need to call it explicitly in the scenario that you are describing.
It isn't going to add any benefit so lets not change the 6.0.x code.
My comment 2 was wrong. I missed that the original code closes oos (which is always null there). It should have closed the file stream instead. oos constructor can throw IOException because it writes stream header (though due to buffering it is unlikely).
Hi, Konstantin, can you explain more about your new comment (Comment 4)? Does the BufferedOutputStream temp object need to be closed in this code piece?
(In reply to comment #5) 1. BufferedOutputStream does not deserve closing, because it is in memory only. No native resources are leaking. - ObjectOutputStream would never need to be closed. If an IOException happens oos will be null as assignment never happens. - The only one that really has to be closed is FileOutputStream. - The only situation when FileOutputStream is not-null and needs to be closed is when its constructor succeeded, but constructors of other streams failed. 2. The only one of other constructors that can (in theory) fail with IOException is ObjectOutputStream. It completes with IOException iff call to ObjectOutputStream#writeStreamHeader() throws an IOException. My Comment 4 was caused by this point "2.". Actually, looking further, the writeStreamHeader() method writes 4 bytes. As those 4 bytes are cached in BufferedOutputStream, an IOException cannot really happen there. So sorry for the noise. It is not worth for 6.0.