diff options
author | Thomas Watson | 2018-02-16 14:59:24 +0000 |
---|---|---|
committer | Thomas Watson | 2018-02-16 22:12:43 +0000 |
commit | 1c1fa3211389fd40c931c5741e42c94807cf9ddd (patch) | |
tree | e5c95cb5bfb4fe2d9583baa42cd9dcba69b478d8 | |
parent | f3adab7540582aac1c7e4aeda29ee6aa64233768 (diff) | |
download | rt.equinox.framework-1c1fa3211389fd40c931c5741e42c94807cf9ddd.tar.gz rt.equinox.framework-1c1fa3211389fd40c931c5741e42c94807cf9ddd.tar.xz rt.equinox.framework-1c1fa3211389fd40c931c5741e42c94807cf9ddd.zip |
Bug 531268 - java.lang.RuntimeException: Must not re-enter openLock inI20180219-2000I20180218-2000I20180217-1500I20180216-2000
getZipFile.
Remove the restriction on re-entering open lock
Change-Id: I7ff625fa7993b58813b4a17f21637931037ac418
Signed-off-by: Thomas Watson <tjwatson@us.ibm.com>
7 files changed, 312 insertions, 150 deletions
diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/SystemBundleTests.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/SystemBundleTests.java index eab1f7794..283667100 100755 --- a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/SystemBundleTests.java +++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/SystemBundleTests.java @@ -16,6 +16,8 @@ import java.io.FileNotFoundException; import java.io.FileOutputStream; import java.io.FileWriter; import java.io.IOException; +import java.io.PrintWriter; +import java.io.StringWriter; import java.net.MalformedURLException; import java.net.Proxy; import java.net.URI; @@ -33,6 +35,7 @@ import java.util.Dictionary; import java.util.Enumeration; import java.util.HashMap; import java.util.Hashtable; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.NoSuchElementException; @@ -43,6 +46,7 @@ import java.util.StringTokenizer; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.CountDownLatch; +import java.util.concurrent.ExecutionException; import java.util.concurrent.ExecutorService; import java.util.concurrent.Executors; import java.util.concurrent.TimeUnit; @@ -53,6 +57,7 @@ import java.util.jar.JarEntry; import java.util.jar.JarOutputStream; import java.util.jar.Manifest; import javax.net.SocketFactory; +import junit.framework.AssertionFailedError; import junit.framework.Test; import junit.framework.TestSuite; import org.eclipse.core.runtime.adaptor.EclipseStarter; @@ -60,11 +65,13 @@ import org.eclipse.equinox.log.ExtendedLogReaderService; import org.eclipse.equinox.log.ExtendedLogService; import org.eclipse.equinox.log.test.TestListener2; import org.eclipse.osgi.framework.util.FilePath; +import org.eclipse.osgi.internal.debug.Debug; import org.eclipse.osgi.internal.framework.EquinoxConfiguration; import org.eclipse.osgi.internal.location.EquinoxLocations; import org.eclipse.osgi.launch.Equinox; import org.eclipse.osgi.service.datalocation.Location; import org.eclipse.osgi.service.environment.EnvironmentInfo; +import org.eclipse.osgi.service.urlconversion.URLConverter; import org.eclipse.osgi.storage.url.reference.Handler; import org.eclipse.osgi.tests.OSGiTestsActivator; import org.eclipse.osgi.tests.security.BaseSecurityTest; @@ -2859,7 +2866,9 @@ public class SystemBundleTests extends AbstractBundleTests { for (Map<String, String> entryMap : entries) { for (Map.Entry<String, String> entry : entryMap.entrySet()) { jos.putNextEntry(new JarEntry(entry.getKey())); - jos.write(entry.getValue().getBytes()); + if (entry.getValue() != null) { + jos.write(entry.getValue().getBytes()); + } jos.closeEntry(); } } @@ -3539,7 +3548,7 @@ public class SystemBundleTests extends AbstractBundleTests { assertEquals("Wrong state for SystemBundle", Bundle.RESOLVED, equinox.getState()); //$NON-NLS-1$ } - public void testMRUBundleFileListOverflow() throws BundleException { + public void testMRUBundleFileListOverflow() throws BundleException, FileNotFoundException, IOException { File config = OSGiTestsActivator.getContext().getDataFile(getName()); //$NON-NLS-1$ final int numBundles = 5000; final File[] testBundles; @@ -3550,26 +3559,27 @@ public class SystemBundleTests extends AbstractBundleTests { throw new RuntimeException(); } + File debugOptions = new File(config, "debugOptions"); + Properties debugProps = new Properties(); + debugProps.setProperty(Debug.OPTION_DEBUG_BUNDLE_FILE, "true"); + FileOutputStream debugOut = new FileOutputStream(debugOptions); + debugProps.store(debugOut, "Equinox Debug Options"); + debugOut.close(); + Map<String, Object> configuration = new HashMap<String, Object>(); configuration.put(EquinoxConfiguration.PROP_FILE_LIMIT, "10"); configuration.put(Constants.FRAMEWORK_STORAGE, config.getAbsolutePath()); + //configuration.put(EquinoxConfiguration.PROP_DEBUG, debugOptions.getAbsolutePath()); final Equinox equinox = new Equinox(configuration); try { - equinox.init(); - } catch (BundleException e) { - fail("Unexpected exception in init()", e); //$NON-NLS-1$ - } - // should be in the STARTING state - assertEquals("Wrong state for SystemBundle", Bundle.STARTING, equinox.getState()); //$NON-NLS-1$ - final BundleContext systemContext = equinox.getBundleContext(); - assertNotNull("System context is null", systemContext); //$NON-NLS-1$ - try { equinox.start(); } catch (BundleException e) { fail("Failed to start the framework", e); //$NON-NLS-1$ } assertEquals("Wrong state for SystemBundle", Bundle.ACTIVE, equinox.getState()); //$NON-NLS-1$ + final BundleContext systemContext = equinox.getBundleContext(); + assertNotNull("System context is null", systemContext); //$NON-NLS-1$ final List<Bundle> bundles = new ArrayList<>(); for (File testBundleFile : testBundles) { @@ -3619,4 +3629,114 @@ public class SystemBundleTests extends AbstractBundleTests { System.out.println(getName() + " time taken: " + timeTaken); assertTrue("Test took too long: " + timeTaken, timeTaken < 30); } + + public void testZipBundleFileOpenLock() throws IOException, BundleException, InvalidSyntaxException { + File config = OSGiTestsActivator.getContext().getDataFile(getName()); //$NON-NLS-1$ + config.mkdirs(); + + Map<String, String> bundleHeaders = new HashMap<String, String>(); + bundleHeaders.put(Constants.BUNDLE_MANIFESTVERSION, "2"); + bundleHeaders.put(Constants.BUNDLE_SYMBOLICNAME, getName()); + Map<String, String> bundleEntries = new LinkedHashMap<>(); + bundleEntries.put("dirA/", null); + bundleEntries.put("dirA/fileA", "fileA"); + bundleEntries.put("dirA/dirB/", null); + bundleEntries.put("dirA/dirB/fileB", "fileB"); + // file in a directory with no directory entry + bundleEntries.put("dirA/dirC/fileC", "fileC"); + File testBundleFile = SystemBundleTests.createBundle(config, getName(), bundleHeaders, bundleEntries); + + Map<String, Object> configuration = new HashMap<String, Object>(); + configuration.put(Constants.FRAMEWORK_STORAGE, config.getAbsolutePath()); + + final Equinox equinox = new Equinox(configuration); + try { + equinox.start(); + } catch (BundleException e) { + fail("Failed to start the framework", e); //$NON-NLS-1$ + } + assertEquals("Wrong state for SystemBundle", Bundle.ACTIVE, equinox.getState()); //$NON-NLS-1$ + final BundleContext systemContext = equinox.getBundleContext(); + + String converterFilter = "(&(objectClass=" + URLConverter.class.getName() + ")(protocol=bundleentry))"; + final URLConverter converter = systemContext.getService(systemContext.getServiceReferences(URLConverter.class, converterFilter).iterator().next()); + + final Bundle testBundle = systemContext.installBundle("file:///" + testBundleFile.getAbsolutePath()); + testBundle.start(); + + final List<FrameworkEvent> errorsAndWarnings = new CopyOnWriteArrayList<FrameworkEvent>(); + FrameworkListener fwkListener = new FrameworkListener() { + @Override + public void frameworkEvent(FrameworkEvent event) { + int type = event.getType(); + if (type == FrameworkEvent.ERROR || type == FrameworkEvent.WARNING) { + errorsAndWarnings.add(event); + } + } + }; + systemContext.addFrameworkListener(fwkListener); + + Runnable asyncTest = new Runnable() { + @Override + public void run() { + try { + assertNotNull("Entry not found.", testBundle.getEntry("dirA/fileA")); + assertNotNull("Entry not found.", testBundle.getEntry("dirA/dirB/fileB")); + assertNotNull("Entry not found.", testBundle.getEntry("dirA/dirC/fileC")); + assertNotNull("Entry not found.", testBundle.getEntry("dirA/dirC/")); + URL dirBURL = converter.toFileURL(testBundle.getEntry("dirA/dirB/")); + assertNotNull("Failed to convert to file URL", dirBURL); + URL dirAURL = converter.toFileURL(testBundle.getEntry("dirA/")); + assertNotNull("Failed to convert to file URL", dirAURL); + List<URL> allEntries = testBundle.adapt(BundleWiring.class).findEntries("/", "*", BundleWiring.FINDENTRIES_RECURSE); + assertEquals("Wrong number of entries: " + allEntries, 8, allEntries.size()); + } catch (IOException e) { + throw new AssertionFailedError(e.getMessage()); + } + } + }; + + // do test with two threads to make sure open lock is not held by a thread + ExecutorService executor1 = Executors.newFixedThreadPool(1); + ExecutorService executor2 = Executors.newFixedThreadPool(1); + try { + executor1.submit(asyncTest).get(); + executor2.submit(asyncTest).get(); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + fail("Interrupted.", e); + } catch (ExecutionException e) { + if (e.getCause() instanceof Error) { + throw (Error) e.getCause(); + } + throw (RuntimeException) e.getCause(); + } finally { + executor1.shutdown(); + executor2.shutdown(); + } + try { + equinox.stop(); + } catch (BundleException e) { + fail("Unexpected erorr stopping framework", e); //$NON-NLS-1$ + } + try { + equinox.waitForStop(10000); + } catch (InterruptedException e) { + fail("Unexpected interrupted exception", e); //$NON-NLS-1$ + } + assertEquals("Wrong state for SystemBundle", Bundle.RESOLVED, equinox.getState()); //$NON-NLS-1$ + + if (!errorsAndWarnings.isEmpty()) { + StringWriter errorStackTraces = new StringWriter(); + PrintWriter writer = new PrintWriter(errorStackTraces); + for (FrameworkEvent frameworkEvent : errorsAndWarnings) { + if (frameworkEvent.getThrowable() != null) { + frameworkEvent.getThrowable().printStackTrace(writer); + } + } + writer.close(); + fail("Found errors or warnings: " + errorsAndWarnings.size() + " - " + errorStackTraces.toString()); + } + } + } diff --git a/bundles/org.eclipse.osgi/.options b/bundles/org.eclipse.osgi/.options index 690325f77..13b000fcb 100644 --- a/bundles/org.eclipse.osgi/.options +++ b/bundles/org.eclipse.osgi/.options @@ -32,7 +32,10 @@ org.eclipse.osgi/debug/objectPool/dups=false org.eclipse.osgi/debug/cachedmanifest = false # Debug the lifecycle operations of the framework/systemBundle org.eclipse.osgi/debug/systemBundle=false +# Debug the framework persistent storage org.eclipse.osgi/debug/storage=false +# Debug the bundle file reading +org.eclipse.osgi/debug/bundleFile=false # Eclipse adaptor options org.eclipse.osgi/eclipseadaptor/debug = false diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/debug/Debug.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/debug/Debug.java index 357fb7b38..5610f9ce5 100644 --- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/debug/Debug.java +++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/debug/Debug.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2003, 2015 IBM Corporation and others. + * Copyright (c) 2003, 2018 IBM Corporation and others. * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -99,6 +99,8 @@ public class Debug implements DebugOptionsListener { public static final String OPTION_DEBUG_SYSTEM_BUNDLE = ECLIPSE_OSGI + "/debug/systemBundle"; //$NON-NLS-1$ + public static final String OPTION_DEBUG_BUNDLE_FILE = ECLIPSE_OSGI + "/debug/bundleFile"; //$NON-NLS-1$ + /** * General debug flag. */ @@ -168,6 +170,8 @@ public class Debug implements DebugOptionsListener { public boolean DEBUG_SYSTEM_BUNDLE = false; // debug/systemBundle + public boolean DEBUG_BUNDLE_FILE = false; // debug/bundleFile + public Debug(DebugOptions dbgOptions) { optionsChanged(dbgOptions); } @@ -194,6 +198,7 @@ public class Debug implements DebugOptionsListener { DEBUG_LOCATION = dbgOptions.getBooleanOption(OPTION_DEBUG_LOCATION, false); DEBUG_CACHED_MANIFEST = dbgOptions.getBooleanOption(OPTION_CACHED_MANIFEST, false); DEBUG_SYSTEM_BUNDLE = dbgOptions.getBooleanOption(OPTION_DEBUG_SYSTEM_BUNDLE, false); + DEBUG_BUNDLE_FILE = dbgOptions.getBooleanOption(OPTION_DEBUG_BUNDLE_FILE, false); } /** diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/Storage.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/Storage.java index be7e283e7..9b89de67b 100644 --- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/Storage.java +++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/Storage.java @@ -177,7 +177,7 @@ public class Storage { } runtimeVersion = javaVersion; javaSpecVersion = javaSpecVersionProp; - mruList = new MRUBundleFileList(getBundleFileLimit(container.getConfiguration())); + mruList = new MRUBundleFileList(getBundleFileLimit(container.getConfiguration()), container.getConfiguration().getDebug()); equinoxContainer = container; extensionInstaller = new FrameworkExtensionInstaller(container.getConfiguration()); allowRestrictedProvides = Boolean.parseBoolean(container.getConfiguration().getConfiguration(EquinoxConfiguration.PROP_ALLOW_RESTRICTED_PROVIDES)); diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/bundlefile/MRUBundleFileList.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/bundlefile/MRUBundleFileList.java index c692402bb..ef876db58 100644 --- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/bundlefile/MRUBundleFileList.java +++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/bundlefile/MRUBundleFileList.java @@ -21,6 +21,7 @@ import java.util.concurrent.locks.ReentrantLock; import org.eclipse.osgi.framework.eventmgr.EventDispatcher; import org.eclipse.osgi.framework.eventmgr.EventManager; import org.eclipse.osgi.framework.eventmgr.ListenerQueue; +import org.eclipse.osgi.internal.debug.Debug; /** * A simple/quick/small implementation of an MRU (Most Recently Used) list to keep @@ -50,10 +51,12 @@ public class MRUBundleFileList implements EventDispatcher<Object, Object, Bundle private final ReentrantLock pendingLock = new ReentrantLock(); private final Condition pendingCond = pendingLock.newCondition(); private final AtomicInteger pending = new AtomicInteger(); + private final Debug debug; - public MRUBundleFileList(int fileLimit) { + public MRUBundleFileList(int fileLimit, Debug debug) { // only enable the MRU if the initFileLimit is > MIN this.fileLimit = fileLimit; + this.debug = debug; if (fileLimit >= MIN) { this.bundleFileList = new BundleFile[fileLimit]; this.useStampList = new long[fileLimit]; @@ -70,15 +73,17 @@ public class MRUBundleFileList implements EventDispatcher<Object, Object, Bundle * the number of open BundleFiles == the fileLimit then the least * recently used BundleFile is closed. * @param bundleFile the bundle file about to be opened. + * @return true if back pressure is needed */ - public void add(BundleFile bundleFile) { + public boolean add(BundleFile bundleFile) { if (fileLimit < MIN) - return; // MRU is disabled + return false; // MRU is disabled BundleFile toRemove = null; EventManager manager = null; + boolean backpressureNeeded = false; synchronized (this) { if (bundleFile.getMruIndex() >= 0) - return; // do nothing; someone is trying add a bundleFile that is already in an MRU list + return false; // do nothing; someone is trying add a bundleFile that is already in an MRU list int index = 0; // default to the first slot if (numOpen < fileLimit) { // numOpen does not exceed the fileLimit @@ -91,7 +96,7 @@ public class MRUBundleFileList implements EventDispatcher<Object, Object, Bundle } else { // numOpen has reached the fileLimit // find the least recently used bundleFile and close it - // and use it slot for the new bundleFile to be opened. + // and use its slot for the new bundleFile to be opened. index = 0; for (int i = 1; i < fileLimit; i++) if (useStampList[i] < useStampList[index]) @@ -100,6 +105,7 @@ public class MRUBundleFileList implements EventDispatcher<Object, Object, Bundle if (toRemove.getMruIndex() != index) throw new IllegalStateException("The BundleFile has the incorrect mru index: " + index + " != " + toRemove.getMruIndex()); //$NON-NLS-1$//$NON-NLS-2$ removeInternal(toRemove); + backpressureNeeded = isBackPressureNeeded(); } // found an index to place to bundleFile to be opened bundleFileList[index] = bundleFile; @@ -116,6 +122,8 @@ public class MRUBundleFileList implements EventDispatcher<Object, Object, Bundle // must not close the toRemove bundle file while holding the lock of another bundle file (bug 161976) // This queues the bundle file for close asynchronously. closeBundleFile(toRemove, manager); + + return backpressureNeeded; } /** @@ -194,21 +202,26 @@ public class MRUBundleFileList implements EventDispatcher<Object, Object, Bundle } } - private void closeBundleFile(BundleFile toRemove, EventManager manager) { - if (toRemove == null) - return; + private boolean isBackPressureNeeded() { pendingLock.lock(); try { - int pendingNum = pending.incrementAndGet(); + return pending.incrementAndGet() > fileLimit; + } finally { + pendingLock.unlock(); + } + } + + public void applyBackpressure() { + pendingLock.lock(); + try { + int pendingNum = pending.get(); if (pendingNum > fileLimit) { - // decrement before waiting since we will not pend until the queue - // reduces a bit - pending.decrementAndGet(); + if (debug.DEBUG_BUNDLE_FILE) { + Debug.println("MRUBundleFileList: Applying back pressure before opening: " + toString()); //$NON-NLS-1$ + } // delay to allow the closer to catchup try { pendingCond.await(Math.min(500, pendingNum), TimeUnit.MILLISECONDS); - // add to the pending count again - pending.incrementAndGet(); } catch (InterruptedException e) { Thread.currentThread().interrupt(); } @@ -216,6 +229,14 @@ public class MRUBundleFileList implements EventDispatcher<Object, Object, Bundle } finally { pendingLock.unlock(); } + } + + private void closeBundleFile(BundleFile toRemove, EventManager manager) { + if (toRemove == null) + return; + if (debug.DEBUG_BUNDLE_FILE) { + Debug.println("MRUBundleFileList: about to close bundle file: " + toRemove); //$NON-NLS-1$ + } try { /* queue to hold set of listeners */ ListenerQueue<Object, Object, BundleFile> queue = new ListenerQueue<>(manager); @@ -227,6 +248,9 @@ public class MRUBundleFileList implements EventDispatcher<Object, Object, Bundle // we cannot propagate exceptions out of this method // failing to queue a bundle close should not cause an error (bug 283797) // TODO should consider logging + if (debug.DEBUG_BUNDLE_FILE) { + Debug.printStackTrace(t); + } } } diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/bundlefile/ZipBundleEntry.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/bundlefile/ZipBundleEntry.java index 00bef5100..598c04ead 100644 --- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/bundlefile/ZipBundleEntry.java +++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/bundlefile/ZipBundleEntry.java @@ -12,7 +12,6 @@ package org.eclipse.osgi.storage.bundlefile; import java.io.File; -import java.io.FilterInputStream; import java.io.IOException; import java.io.InputStream; import java.net.MalformedURLException; @@ -51,20 +50,7 @@ public class ZipBundleEntry extends BundleEntry { * @exception java.io.IOException */ public InputStream getInputStream() throws IOException { - ZipBundleFile zipBundleFile = bundleFile; - - if (!zipBundleFile.isMruEnabled()) - return bundleFile.getZipFile(false).getInputStream(zipEntry); - - zipBundleFile.incrementReference(); - InputStream result = null; - try { - return result = new ZipBundleEntryInputStream(zipBundleFile.getZipFile(false).getInputStream(zipEntry)); - } finally { - if (result == null) - // an exception occurred; decrement the reference - zipBundleFile.decrementReference(); - } + return bundleFile.getInputStream(zipEntry); } /** @@ -117,80 +103,4 @@ public class ZipBundleEntry extends BundleEntry { } return null; } - - private class ZipBundleEntryInputStream extends FilterInputStream { - - private boolean closed = false; - - public ZipBundleEntryInputStream(InputStream stream) { - super(stream); - } - - public int available() throws IOException { - try { - return super.available(); - } catch (IOException e) { - throw enrichExceptionWithBaseFile(e); - } - } - - public void close() throws IOException { - try { - super.close(); - } catch (IOException e) { - throw enrichExceptionWithBaseFile(e); - } finally { - synchronized (this) { - if (closed) - return; - closed = true; - } - bundleFile.decrementReference(); - } - } - - public int read() throws IOException { - try { - return super.read(); - } catch (IOException e) { - throw enrichExceptionWithBaseFile(e); - } - } - - public int read(byte[] var0, int var1, int var2) throws IOException { - try { - return super.read(var0, var1, var2); - } catch (IOException e) { - throw enrichExceptionWithBaseFile(e); - } - } - - public int read(byte[] var0) throws IOException { - try { - return super.read(var0); - } catch (IOException e) { - throw enrichExceptionWithBaseFile(e); - } - } - - public void reset() throws IOException { - try { - super.reset(); - } catch (IOException e) { - throw enrichExceptionWithBaseFile(e); - } - } - - public long skip(long var0) throws IOException { - try { - return super.skip(var0); - } catch (IOException e) { - throw enrichExceptionWithBaseFile(e); - } - } - - private IOException enrichExceptionWithBaseFile(IOException e) { - return new IOException(bundleFile.getBaseFile().toString(), e); - } - } } diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/bundlefile/ZipBundleFile.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/bundlefile/ZipBundleFile.java index a865929f6..915f5a47d 100644 --- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/bundlefile/ZipBundleFile.java +++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/bundlefile/ZipBundleFile.java @@ -13,6 +13,7 @@ package org.eclipse.osgi.storage.bundlefile; import java.io.File; +import java.io.FilterInputStream; import java.io.IOException; import java.io.InputStream; import java.util.Collections; @@ -98,15 +99,6 @@ public class ZipBundleFile extends BundleFile { } /** - * Opens the ZipFile for this bundle file - * @return an open ZipFile for this bundle file - * @throws IOException - */ - private ZipFile basicOpen() throws IOException { - return BundleFile.secureAction.getZipFile(this.basefile); - } - - /** * Returns an open ZipFile for this bundle file. If an open * ZipFile does not exist then a new one is created and * returned. @@ -114,37 +106,45 @@ public class ZipBundleFile extends BundleFile { * @return an open ZipFile for this bundle * @throws IOException */ - ZipFile getZipFile(boolean keepLock) throws IOException { - if (openLock.getHoldCount() != 0) { - ModuleRevision r = generation.getRevision(); - if (r != null) { - // post an error about reentrant lock here - RuntimeException error = new RuntimeException("Must not re-enter openLock in getZipFile."); //$NON-NLS-1$ - generation.getBundleInfo().getStorage().getAdaptor().publishContainerEvent(ContainerEvent.ERROR, r.getRevisions().getModule(), error); - } - } - if (closed) { - // do this outside the lock - mruListAdd(); - } + private ZipFile getZipFile(boolean keepLock) throws IOException { openLock.lock(); try { if (closed) { - zipFile = basicOpen(); - closed = false; + boolean needBackPressure = mruListAdd(); + if (needBackPressure) { + // release lock before applying back pressure + openLock.unlock(); + try { + mruListApplyBackPressure(); + } finally { + // get lock back after back pressure + openLock.lock(); + } + } + // check close again after getting open lock again + if (closed) { + // always add again if back pressure was applied in case + // the bundle file got removed while releasing the open lock + if (needBackPressure) { + mruListAdd(); + } + // This can throw an IO exception resulting in closed remaining true on exit + zipFile = BundleFile.secureAction.getZipFile(this.basefile); + closed = false; + } } else { mruListUse(); } return zipFile; } finally { - if (!keepLock || zipFile == null) { + if (!keepLock || closed) { openLock.unlock(); } } } /** - * Returns a ZipEntry for the bundle file. Must be called while holding the monitor lock. + * Returns a ZipEntry for the bundle file. Must be called while holding the open lock. * This method does not ensure that the ZipFile is opened. Callers may need to call getZipfile() prior to calling this * method. * @param path the path to an entry @@ -213,7 +213,7 @@ public class ZipBundleFile extends BundleFile { if (nested != null) { if (nested.exists()) { /* the entry is already cached */ - if (debug.DEBUG_GENERAL) + if (debug.DEBUG_BUNDLE_FILE) Debug.println("File already present: " + nested.getPath()); //$NON-NLS-1$ if (nested.isDirectory()) // must ensure the complete directory is extracted (bug 182585) @@ -222,7 +222,7 @@ public class ZipBundleFile extends BundleFile { if (zipEntry.getName().endsWith("/")) { //$NON-NLS-1$ nested.mkdirs(); if (!nested.isDirectory()) { - if (debug.DEBUG_GENERAL) + if (debug.DEBUG_BUNDLE_FILE) Debug.println("Unable to create directory: " + nested.getPath()); //$NON-NLS-1$ throw new IOException(NLS.bind(Msg.ADAPTOR_DIRECTORY_CREATE_EXCEPTION, nested.getAbsolutePath())); } @@ -238,7 +238,7 @@ public class ZipBundleFile extends BundleFile { return nested; } } catch (IOException e) { - if (debug.DEBUG_GENERAL) + if (debug.DEBUG_BUNDLE_FILE) Debug.printStackTrace(e); generation.getBundleInfo().getStorage().getLogServices().log(EquinoxContainer.NAME, FrameworkLogEntry.ERROR, "Unable to extract content: " + generation.getRevision() + ": " + entry, e); //$NON-NLS-1$ //$NON-NLS-2$ } @@ -380,6 +380,7 @@ public class ZipBundleFile extends BundleFile { closed = true; zipFile.close(); mruListRemove(); + zipFile = null; } } finally { openLock.unlock(); @@ -390,7 +391,7 @@ public class ZipBundleFile extends BundleFile { return this.mruList != null && this.mruList.isClosing(this); } - boolean isMruEnabled() { + private boolean isMruEnabled() { return this.mruList != null && this.mruList.isEnabled(); } @@ -406,12 +407,19 @@ public class ZipBundleFile extends BundleFile { } } - private void mruListAdd() { + private void mruListApplyBackPressure() { if (this.mruList != null) { - mruList.add(this); + this.mruList.applyBackpressure(); } } + private boolean mruListAdd() { + if (this.mruList != null) { + return mruList.add(this); + } + return false; + } + public void open() throws IOException { getZipFile(false); } @@ -436,4 +444,96 @@ public class ZipBundleFile extends BundleFile { openLock.unlock(); } } + + InputStream getInputStream(ZipEntry entry) throws IOException { + if (!lockOpen()) { + throw new IOException("Failed to open zip file."); //$NON-NLS-1$ + } + try { + InputStream zipStream = zipFile.getInputStream(entry); + if (isMruEnabled()) { + zipStream = new ZipBundleEntryInputStream(zipStream); + } + return zipStream; + } finally { + openLock.unlock(); + } + } + + private class ZipBundleEntryInputStream extends FilterInputStream { + + private boolean streamClosed = false; + + public ZipBundleEntryInputStream(InputStream stream) { + super(stream); + incrementReference(); + } + + public int available() throws IOException { + try { + return super.available(); + } catch (IOException e) { + throw enrichExceptionWithBaseFile(e); + } + } + + public void close() throws IOException { + try { + super.close(); + } catch (IOException e) { + throw enrichExceptionWithBaseFile(e); + } finally { + synchronized (this) { + if (streamClosed) + return; + streamClosed = true; + } + decrementReference(); + } + } + + public int read() throws IOException { + try { + return super.read(); + } catch (IOException e) { + throw enrichExceptionWithBaseFile(e); + } + } + + public int read(byte[] var0, int var1, int var2) throws IOException { + try { + return super.read(var0, var1, var2); + } catch (IOException e) { + throw enrichExceptionWithBaseFile(e); + } + } + + public int read(byte[] var0) throws IOException { + try { + return super.read(var0); + } catch (IOException e) { + throw enrichExceptionWithBaseFile(e); + } + } + + public void reset() throws IOException { + try { + super.reset(); + } catch (IOException e) { + throw enrichExceptionWithBaseFile(e); + } + } + + public long skip(long var0) throws IOException { + try { + return super.skip(var0); + } catch (IOException e) { + throw enrichExceptionWithBaseFile(e); + } + } + + private IOException enrichExceptionWithBaseFile(IOException e) { + return new IOException(getBaseFile().toString(), e); + } + } } |