Bug 449779 - Deadlock within the OSGi framework

 - Introduce  and use AtomicLazyInitializer to manage construction of the loader
 - Remove the monitor lock from ModuleWiring

2 files changed, 99 insertions, 27 deletions

diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/ModuleWiring.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/ModuleWiring.java
index 53d08b27e..b881b462d 100644
--- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/ModuleWiring.java
+++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/ModuleWiring.java
@@ -12,8 +12,10 @@ package org.eclipse.osgi.container;
 
 import java.net.URL;
 import java.util.*;
+import java.util.concurrent.Callable;
 import java.util.concurrent.atomic.AtomicReference;
 import org.eclipse.osgi.container.ModuleRevisionBuilder.GenericInfo;
+import org.eclipse.osgi.internal.container.AtomicLazyInitializer;
 import org.eclipse.osgi.internal.container.InternalUtils;
 import org.osgi.framework.AdminPermission;
 import org.osgi.framework.Bundle;
@@ -27,17 +29,27 @@ import org.osgi.resource.*;
  * @since 3.10
  */
 public final class ModuleWiring implements BundleWiring {
+	class LoaderInitializer implements Callable<ModuleLoader> {
+		@Override
+		public ModuleLoader call() throws Exception {
+			if (!isValid) {
+				return null;
+			}
+			return getRevision().getRevisions().getContainer().adaptor.createModuleLoader(ModuleWiring.this);
+		}
+	}
+
 	private static final RuntimePermission GET_CLASSLOADER_PERM = new RuntimePermission("getClassLoader"); //$NON-NLS-1$
 	private static final String DYNAMICALLY_ADDED_IMPORT_DIRECTIVE = "x.dynamically.added"; //$NON-NLS-1$
 	private final ModuleRevision revision;
 	private volatile List<ModuleCapability> capabilities;
 	private volatile List<ModuleRequirement> requirements;
 	private final Collection<String> substitutedPkgNames;
-	private final Object monitor = new Object();
-	private ModuleLoader loader = null;
+	private final AtomicLazyInitializer<ModuleLoader> loader = new AtomicLazyInitializer<ModuleLoader>();
+	private final LoaderInitializer loaderInitializer = new LoaderInitializer();
 	private volatile List<ModuleWire> providedWires;
 	private volatile List<ModuleWire> requiredWires;
-	private volatile boolean isValid = true;
+	volatile boolean isValid = true;
 	private final AtomicReference<Set<String>> dynamicMissRef = new AtomicReference<Set<String>>();
 
 	ModuleWiring(ModuleRevision revision, List<ModuleCapability> capabilities, List<ModuleRequirement> requirements, List<ModuleWire> providedWires, List<ModuleWire> requiredWires, Collection<String> substitutedPkgNames) {
@@ -251,23 +263,14 @@ public final class ModuleWiring implements BundleWiring {
 	 * @return the module loader for this wiring.
 	 */
 	public ModuleLoader getModuleLoader() {
-		synchronized (monitor) {
-			if (loader == null) {
-				if (!isValid) {
-					return null;
-				}
-				loader = revision.getRevisions().getContainer().adaptor.createModuleLoader(this);
-			}
-			return loader;
-		}
+		return loader.getInitialized(loaderInitializer);
 
 	}
 
 	void loadFragments(Collection<ModuleRevision> fragments) {
-		synchronized (monitor) {
-			if (loader != null) {
-				loader.loadFragments(fragments);
-			}
+		ModuleLoader current = loader.get();
+		if (current != null) {
+			current.loadFragments(fragments);
 		}
 	}
 
@@ -351,21 +354,14 @@ public final class ModuleWiring implements BundleWiring {
 	}
 
 	private void invalidate0(boolean releaseLoader) {
-		ModuleLoader current;
-		synchronized (monitor) {
-			this.isValid = false;
-			current = loader;
-			if (releaseLoader) {
-				loader = null;
-			}
-		}
+		// set the isValid to false first
+		isValid = false;
+		ModuleLoader current = releaseLoader ? loader.getAndClear() : loader.get();
 		revision.getRevisions().getContainer().getAdaptor().invalidateWiring(this, current);
 	}
 
 	void validate() {
-		synchronized (monitor) {
-			this.isValid = true;
-		}
+		this.isValid = true;
 	}
 
 	boolean isSubtituted(ModuleCapability capability) {
diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/container/AtomicLazyInitializer.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/container/AtomicLazyInitializer.java
new file mode 100644
index 000000000..bb72070d5
--- /dev/null
+++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/internal/container/AtomicLazyInitializer.java
@@ -0,0 +1,76 @@
+/*******************************************************************************
+ * Copyright (c) 2014 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
+ * http://www.eclipse.org/legal/epl-v10.html
+ * 
+ * Contributors:
+ *     IBM Corporation - initial API and implementation
+ *******************************************************************************/
+package org.eclipse.osgi.internal.container;
+
+import java.util.concurrent.Callable;
+import java.util.concurrent.atomic.AtomicReference;
+
+/**
+ * A helper class for doing lazy initialization 
+ * 
+ * @param <V> the type of object to lazy initialize
+ */
+public class AtomicLazyInitializer<V> {
+	private final AtomicReference<V> holder = new AtomicReference<V>();
+
+	/**
+	 * Gets the current value.  If the value has not been initialized then
+	 * {@code null} is returned;
+	 * @return the current value
+	 */
+	public final V get() {
+		return holder.get();
+	}
+
+	/**
+	 * Atomically gets the current initialized value.  If the current value is {@code null}
+	 * then the supplied initializer is called to create the value returned.
+	 * @param initializer the initializer to call if the current value is {@code null}
+	 * @return the initialized value.  May return {@code null} if initializer returns null.
+	 */
+	public final V getInitialized(Callable<V> initializer) {
+		V result = holder.get();
+		if (result != null) {
+			return result;
+		}
+		// Must hold a lock to ensure the operation is atomic.
+		synchronized (holder) {
+			result = holder.get();
+			if (result != null) {
+				return result;
+			}
+			try {
+				result = initializer.call();
+			} catch (Exception e) {
+				unchecked(e);
+			}
+			holder.set(result);
+			return result;
+		}
+	}
+
+	/**
+	 * Gets the current value and clears the value for future calls to this lazy initializer.
+	 * @return the current value
+	 */
+	public final V getAndClear() {
+		return holder.getAndSet(null);
+	}
+
+	private static <T> T unchecked(Exception exception) {
+		return AtomicLazyInitializer.<T, RuntimeException> unchecked0(exception);
+	}
+
+	@SuppressWarnings("unchecked")
+	private static <T, E extends Exception> T unchecked0(Exception exception) throws E {
+		throw (E) exception;
+	}
+}
\ No newline at end of file