Bug 431363 - [coordinator] unexpected orphaned failures may happen when

ending a coordination

Change-Id: I6261e34228d63c71df3f381b36bc7fb4c2ab5219
Signed-off-by: Thomas Watson <tjwatson@us.ibm.com>

2 files changed, 41 insertions, 17 deletions

diff --git a/bundles/org.eclipse.equinox.coordinator/src/org/eclipse/equinox/coordinator/CoordinationImpl.java b/bundles/org.eclipse.equinox.coordinator/src/org/eclipse/equinox/coordinator/CoordinationImpl.java
index 2ad246c99..ceeef4273 100644
--- a/bundles/org.eclipse.equinox.coordinator/src/org/eclipse/equinox/coordinator/CoordinationImpl.java
+++ b/bundles/org.eclipse.equinox.coordinator/src/org/eclipse/equinox/coordinator/CoordinationImpl.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2010, 2013 IBM Corporation and others.
+ * Copyright (c) 2010, 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
@@ -33,7 +33,8 @@ public class CoordinationImpl {
 
 	private volatile Throwable failure;
 	private volatile boolean terminated;
-
+	private volatile boolean ending = false;
+	
 	private Date deadline;
 	private CoordinationImpl enclosingCoordination;
 	private Thread thread;
@@ -128,6 +129,23 @@ public class CoordinationImpl {
 		coordinator.checkPermission(CoordinationPermission.INITIATE, name);
 		// Terminating the coordination must be atomic.
 		synchronized (this) {
+			/*
+			 * Set the ending flag to avoid spurious failures for orphans
+			 * It appears the VM can aggressively puts objects on the queue if the last call is done in a finally
+			 * Coordination c = coordinator.begin("name", 0);
+			 * try {
+			 *   ...
+			 * } finally {
+			 *   c.end()
+			 * }
+			 * In some cases it appears that while in the finally call to c.end()
+			 * that c can become put on the queue for GC.
+			 * This makes it eligible for orphan processing which will cause 
+			 * issues below when calling methods that invoke
+			 * CoordinationWeakReference.processOrphanedCoordinations()
+			 * We set an ending flag so that we can detect this
+			 */
+			ending = true;
 			// If this coordination is associated with a thread, an additional
 			// check is required.
 			if (thread != null) {
@@ -335,6 +353,10 @@ public class CoordinationImpl {
 		return terminated;
 	}
 
+	public boolean isEnding() {
+		return ending;
+	}
+
 	public void join(final long timeInMillis) throws InterruptedException {
 		coordinator.checkPermission(CoordinationPermission.PARTICIPATE, name);
 		validateTimeout(timeInMillis);
diff --git a/bundles/org.eclipse.equinox.coordinator/src/org/eclipse/equinox/coordinator/CoordinationWeakReference.java b/bundles/org.eclipse.equinox.coordinator/src/org/eclipse/equinox/coordinator/CoordinationWeakReference.java
index 33d5ef4ae..7ffb66e0e 100644
--- a/bundles/org.eclipse.equinox.coordinator/src/org/eclipse/equinox/coordinator/CoordinationWeakReference.java
+++ b/bundles/org.eclipse.equinox.coordinator/src/org/eclipse/equinox/coordinator/CoordinationWeakReference.java
@@ -1,5 +1,5 @@
 /*******************************************************************************
- * Copyright (c) 2011, 2013 IBM Corporation and others.
+ * Copyright (c) 2011, 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
@@ -25,25 +25,27 @@ public class CoordinationWeakReference extends WeakReference<CoordinationReferen
 		CoordinationWeakReference r;
 		while ((r = (CoordinationWeakReference)referenceQueue.poll()) != null) {
 			CoordinationImpl c = r.getCoordination();
-			try {
-				c.fail(Coordination.ORPHANED);
-			}
-			catch (Exception e) {
-				c.getLogService().log(LogService.LOG_WARNING, NLS.bind(Messages.OrphanedCoordinationError, c.getName(), c.getId()), e);
-			}
-			finally {
+			if (!c.isEnding()) {
 				try {
-					c.end();
-				}
-				catch (CoordinationException e) {
-					// This is expected since we already failed the coordination...
-					if (!Coordination.ORPHANED.equals(e.getCause()))
-						// ...but only if the cause is ORPHANED.
-						c.getLogService().log(LogService.LOG_DEBUG, NLS.bind(Messages.OrphanedCoordinationError, c.getName(), c.getId()), e);
+					c.fail(Coordination.ORPHANED);
 				}
 				catch (Exception e) {
 					c.getLogService().log(LogService.LOG_WARNING, NLS.bind(Messages.OrphanedCoordinationError, c.getName(), c.getId()), e);
 				}
+				finally {
+					try {
+						c.end();
+					}
+					catch (CoordinationException e) {
+						// This is expected since we already failed the coordination...
+						if (!Coordination.ORPHANED.equals(e.getCause()))
+							// ...but only if the cause is ORPHANED.
+							c.getLogService().log(LogService.LOG_DEBUG, NLS.bind(Messages.OrphanedCoordinationError, c.getName(), c.getId()), e);
+					}
+					catch (Exception e) {
+						c.getLogService().log(LogService.LOG_WARNING, NLS.bind(Messages.OrphanedCoordinationError, c.getName(), c.getId()), e);
+					}
+				}
 			}
 		}
 	}