Community
Participate
Working Groups
From bug #300589, Stephan found the following: OK, since this bug is still relevant, I had a quick look at the source of org.eclipse.osgi.internal.signedcontent.SignatureBlockProcessor.readIntoArray(BundleEntry) It looks like a very simply file handle leak here: the InputStream "is" might be allocated in be.getInputStream() but is never closed. Looks like a simple } finally { if (is != null) is.close(); } might solve the issue at this location (there might be more!!). I'm pretty confident that this helps because I previously had the same bug pattern in my code and the finally did fix one instance of "Too many open files" :) Then in a second comments he adds: More evidence: the error message in bug 349073 reads "Error reading signed content". That's logged from org.eclipse.equinox.internal.p2.artifact.repository.SignatureVerifier.verifyContent() When drilling into verifierFactory.getSignedContent(inputFile); we actually get to the same readIntoArray() method mentioned in my previous comment.
Created attachment 197885 [details] patch
Thanks for the find Stephan! I released a fix to HEAD. I will open a separate bug to fix for Indigo SR1.
fixed.
(In reply to comment #2) > Thanks for the find Stephan! welcome :) > I released a fix to HEAD. Regarding your patch: Are you sure "is" will *never* be null? Would be a pity if the method throws just during cleanup, no? I seem to see implementors of BundleEntry.getInputStream() that can indeed return null. Also: how about IOException during close? Just wondering..
Created attachment 197886 [details] updated patch (In reply to comment #4) > (In reply to comment #2) > > Thanks for the find Stephan! > > welcome :) > > > I released a fix to HEAD. > > Regarding your patch: Are you sure "is" will *never* be null? BundleEntry object should never return null here. An IOException must be thrown if the entry cannot produce an InputStream. > Would be a pity if the method throws just during cleanup, no? > I seem to see implementors of BundleEntry.getInputStream() > that can indeed return null. Which implementors? I found DirZipBundleEntry returned null, incorrectly and is fixed in this patch. Are there others? > Also: how about IOException during close? > Just wondering.. Here is an update patch that catches the exception on close.
(In reply to comment #5) > > Regarding your patch: Are you sure "is" will *never* be null? > > BundleEntry object should never return null here. An IOException must be > thrown if the entry cannot produce an InputStream. Great, so we know what *should* happen. > > Would be a pity if the method throws just during cleanup, no? > > I seem to see implementors of BundleEntry.getInputStream() > > that can indeed return null. > > Which implementors? I found DirZipBundleEntry returned null, incorrectly and > is fixed in this patch. Are there others? Actually ZipBundleEntry returns ZipFile.getInputStream() which is "internally documented" to return null for non-existing entries (when looking at JDK < 1.7: via the private getInputStream(String))
(In reply to comment #6) > > Actually ZipBundleEntry returns ZipFile.getInputStream() which is > "internally documented" to return null for non-existing entries > (when looking at JDK < 1.7: via the private getInputStream(String)) This should not happen since we only construct ZipBundleEntry objects after confirming that the entry actually exists in the ZipFile.
FYI, the latest patch was released. I am reluctant to do any null checks for inputstreams here since we make the assumption all over in the framework that null is never returned here.
