Bug 559634 - [OSGi R8] ConnectBundleFile.getConnectHeaders() calls

ConnectContent.getHeaders() without ensuring open is called.

Change-Id: Ia2a96fa74a364f5a1d4270a3b450c23b016abe23
Signed-off-by: Anjum Fatima <anjum.eclipse@gmail.com>

3 files changed, 68 insertions, 8 deletions

diff --git a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/ConnectTests.java b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/ConnectTests.java
index 086550c82..0220f6533 100644
--- a/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/ConnectTests.java
+++ b/bundles/org.eclipse.osgi.tests/src/org/eclipse/osgi/tests/bundles/ConnectTests.java
@@ -717,6 +717,52 @@ public class ConnectTests extends AbstractBundleTests {
 		}, withSignedHook);
 	}
 
+	public void testGetConnectHeaders() throws Exception {
+		final String NAME = "bundle";
+		final AtomicReference<Dictionary<String, String>> headers1 = new AtomicReference<>();
+		final AtomicReference<Dictionary<String, String>> headers2 = new AtomicReference<>();
+
+		TestCountingConnectFramework connectFactory = new TestCountingConnectFramework();
+		TestConnectModule m = createSimpleHeadersModule(NAME);
+		connectFactory.setModule(NAME, m);
+		doTestConnect(connectFactory, new HashMap<>(), (f) -> {
+			try {
+				f.start();
+				Bundle b = f.getBundleContext().installBundle(NAME);
+				assertEquals("Wrong name.", NAME, b.getSymbolicName());
+				headers1.set(b.getHeaders());
+				f.stop();
+				f.waitForStop(5000);
+			} catch (Throwable t) {
+				sneakyThrow(t);
+			}
+		});
+
+		doTestConnect(connectFactory, new HashMap<>(), (f) -> {
+			try {
+				f.start();
+				Bundle b = f.getBundleContext().getBundle(NAME);
+				assertFalse("Content is not closed", m.getContent().isOpen());
+				//Bundle.getHeaders() will eventually call ConnectBundleFile.getConnectHeaders() which opens the connect content
+				headers2.set(b.getHeaders());
+				assertTrue("Content is not open", m.getContent().isOpen());
+				f.stop();
+				f.waitForStop(5000);
+			} catch (Throwable t) {
+				sneakyThrow(t);
+			}
+		});
+		Dictionary<String, String> h1 = headers1.get();
+		Dictionary<String, String> h2 = headers2.get();
+
+		assertEquals("Headers size not equal", h1.size(), h2.size());
+
+		for (Enumeration<String> keys = h1.keys(); keys.hasMoreElements();) {
+			String key = keys.nextElement();
+			assertEquals(key + " header value not equal", h1.get(key), h2.get(key));
+		}
+	}
+
 	private void checkHeaders(Map<String, String> expected, Dictionary<String, String> actual) {
 		assertEquals("Headers size not equals", expected.size(), actual.size());
 		for (Entry<String, String> entry : expected.entrySet()) {
diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/connect/ConnectBundleFile.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/connect/ConnectBundleFile.java
index 37e38cb17..25703120d 100644
--- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/connect/ConnectBundleFile.java
+++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/connect/ConnectBundleFile.java
@@ -128,7 +128,14 @@ public class ConnectBundleFile extends CloseableBundleFile<ConnectEntry> {
 	}
 
 	public Map<String, String> getConnectHeaders() {
-		return content.getHeaders().orElse(null);
+		if (!lockOpen()) {
+			return null;
+		}
+		try {
+			return content.getHeaders().orElse(null);
+		} finally {
+			releaseOpen();
+		}
 	}
 
 	Optional<ClassLoader> getClassLoader() {
diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/bundlefile/CloseableBundleFile.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/bundlefile/CloseableBundleFile.java
index f37d25f08..371e2364b 100644
--- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/bundlefile/CloseableBundleFile.java
+++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/storage/bundlefile/CloseableBundleFile.java
@@ -72,7 +72,7 @@ public abstract class CloseableBundleFile<E> extends BundleFile {
 	 * Checks if the bundle file is open
 	 * @return true if the bundle file is open and locked
 	 */
-	private boolean lockOpen() {
+	protected boolean lockOpen() {
 		try {
 			open(true);
 			return true;
@@ -98,6 +98,13 @@ public abstract class CloseableBundleFile<E> extends BundleFile {
 	}
 
 	/**
+	 * Unlocks the open lock
+	 */
+	protected void releaseOpen() {
+		openLock.unlock();
+	}
+
+	/**
 	 * Opens this bundle file.
 	 * @param keepLock true if the open lock should be retained
 	 * @throws IOException
@@ -162,7 +169,7 @@ public abstract class CloseableBundleFile<E> extends BundleFile {
 			}
 			return getExtractFile(dirName);
 		} finally {
-			openLock.unlock();
+			releaseOpen();
 		}
 	}
 
@@ -219,7 +226,7 @@ public abstract class CloseableBundleFile<E> extends BundleFile {
 				generation.getBundleInfo().getStorage().getLogServices().log(EquinoxContainer.NAME, FrameworkLogEntry.ERROR, "Unable to extract content: " + generation.getRevision() + ": " + entry, e); //$NON-NLS-1$ //$NON-NLS-2$
 			}
 		} finally {
-			openLock.unlock();
+			releaseOpen();
 		}
 		return null;
 	}
@@ -251,7 +258,7 @@ public abstract class CloseableBundleFile<E> extends BundleFile {
 				}
 			}
 		} finally {
-			openLock.unlock();
+			releaseOpen();
 		}
 		return false;
 	}
@@ -264,7 +271,7 @@ public abstract class CloseableBundleFile<E> extends BundleFile {
 		try {
 			return findEntry(path);
 		} finally {
-			openLock.unlock();
+			releaseOpen();
 		}
 	}
 
@@ -307,7 +314,7 @@ public abstract class CloseableBundleFile<E> extends BundleFile {
 			}
 			return result.size() == 0 ? null : Collections.enumeration(result);
 		} finally {
-			openLock.unlock();
+			releaseOpen();
 		}
 	}
 
@@ -449,7 +456,7 @@ public abstract class CloseableBundleFile<E> extends BundleFile {
 			}
 			return in;
 		} finally {
-			openLock.unlock();
+			releaseOpen();
 		}
 	}