From 8e2dbcef77350dd8075a84fd3b30b3bc45c9388f Mon Sep 17 00:00:00 2001 From: Thomas Watson Date: Thu, 7 Jul 2016 11:24:12 -0500 Subject: Bug 416164 - Correctly handle spurious wake ups. Replace forStop may with a single AtomicReference that we can hand out to waiting threads and notify when the framework stops Change-Id: I1f6da6b9c896ba1bc2397bccfd9b34899a497bce Signed-off-by: Thomas Watson --- .../org/eclipse/osgi/container/SystemModule.java | 101 ++++++++++++--------- 1 file changed, 59 insertions(+), 42 deletions(-) diff --git a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/SystemModule.java b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/SystemModule.java index df498f62f..0f4b65e2b 100644 --- a/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/SystemModule.java +++ b/bundles/org.eclipse.osgi/container/src/org/eclipse/osgi/container/SystemModule.java @@ -10,8 +10,10 @@ *******************************************************************************/ package org.eclipse.osgi.container; -import java.util.*; +import java.util.Arrays; +import java.util.EnumSet; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicReference; import org.eclipse.osgi.container.ModuleContainer.ContainerStartLevel; import org.eclipse.osgi.container.ModuleContainerAdaptor.ContainerEvent; import org.eclipse.osgi.container.ModuleContainerAdaptor.ModuleEvent; @@ -22,13 +24,13 @@ import org.osgi.framework.launch.Framework; import org.osgi.service.resolver.ResolutionException; /** - * A special kind of module that represents the system module for the container. Additinal + * A special kind of module that represents the system module for the container. Additional * methods are available on the system module for operations that effect the whole container. * For example, initializing the container, restarting and waiting for the container to stop. * @since 3.10 */ public abstract class SystemModule extends Module { - private final Map forStop = new HashMap(2); + private volatile AtomicReference forStop = new AtomicReference(); public SystemModule(ModuleContainer container) { super(new Long(0), Constants.SYSTEM_BUNDLE_LOCATION, container, EnumSet.of(Settings.AUTO_START, Settings.USE_ACTIVATION_POLICY), Integer.valueOf(0)); @@ -82,6 +84,11 @@ public abstract class SystemModule extends Module { } setState(State.STARTING); + AtomicReference existingForStop = forStop; + if (existingForStop.get() != null) { + // There was a previous launch, reset the reference forStop + forStop = new AtomicReference(); + } publishEvent(ModuleEvent.STARTING); try { initWorker(); @@ -116,49 +123,53 @@ public abstract class SystemModule extends Module { public ContainerEvent waitForStop(long timeout) throws InterruptedException { final boolean waitForever = timeout == 0; final long start = System.currentTimeMillis(); - final Thread current = Thread.currentThread(); long timeLeft = timeout; - ContainerEvent event = null; - boolean stateLocked; - if (timeout == 0) { - stateChangeLock.lockInterruptibly(); - stateLocked = true; - } else { - stateLocked = stateChangeLock.tryLock(timeLeft, TimeUnit.MILLISECONDS); - } - if (stateLocked) { - synchronized (forStop) { - try { - if (!Module.ACTIVE_SET.contains(getState())) { - return ContainerEvent.STOPPED; - } - event = forStop.remove(current); - if (event == null) { - forStop.put(current, null); - } - } finally { - stateChangeLock.unlock(); - } - timeLeft = waitForever ? 0 : start + timeout - System.currentTimeMillis(); - while (event == null && (waitForever || timeLeft > 0)) { - forStop.wait(timeLeft); - event = forStop.remove(current); - if (!waitForever) { - timeLeft = start + timeout - System.currentTimeMillis(); - } - } + AtomicReference stopEvent = null; + State currentState = null; + boolean stateLocked = false; + try { + if (timeout == 0) { + stateChangeLock.lockInterruptibly(); + stateLocked = true; + } else { + stateLocked = stateChangeLock.tryLock(timeLeft, TimeUnit.MILLISECONDS); + } + if (stateLocked) { + stopEvent = forStop; + currentState = getState(); + } + } finally { + if (stateLocked) { + stateChangeLock.unlock(); } } - return event == null ? ContainerEvent.STOPPED_TIMEOUT : event; - } - private void notifyWaitForStop(ContainerEvent event) { - synchronized (forStop) { - Collection waiting = new ArrayList(forStop.keySet()); - for (Thread t : waiting) { - forStop.put(t, event); + if (stopEvent == null || currentState == null) { + // Could not lock system module stateChangeLock; timeout + return ContainerEvent.STOPPED_TIMEOUT; + } + if (!ACTIVE_SET.contains(currentState)) { + // check if a past event is waiting for us + ContainerEvent result = stopEvent.get(); + if (result != null) { + return result; } - forStop.notifyAll(); + // framework must not have even been started yet + return ContainerEvent.STOPPED; + } + synchronized (stopEvent) { + do { + ContainerEvent result = stopEvent.get(); + if (result != null) { + return result; + } + timeLeft = waitForever ? 0 : start + timeout - System.currentTimeMillis(); + if (waitForever || timeLeft > 0) { + stopEvent.wait(timeLeft); + } else { + return ContainerEvent.STOPPED_TIMEOUT; + } + } while (true); } } @@ -203,15 +214,21 @@ public abstract class SystemModule extends Module { getRevisions().getContainer().adaptor.publishContainerEvent(containerEvent, this, null); getRevisions().getContainer().close(); } finally { + AtomicReference eventReference = forStop; + eventReference.compareAndSet(null, containerEvent); stateChangeLock.unlock(); + synchronized (eventReference) { + eventReference.notifyAll(); + } } } else { throw new BundleException(Msg.SystemModule_LockError); } } catch (InterruptedException e) { getRevisions().getContainer().adaptor.publishContainerEvent(ContainerEvent.ERROR, this, e); + throw new BundleException(Msg.Module_LockError + toString(), BundleException.STATECHANGE_ERROR, e); } - notifyWaitForStop(containerEvent); + } /** -- cgit v1.2.1