Fixed race condition/hang for cancelation; added pinTable, also made handling of state detail on cancelation consistent by adding CANCELED state detal to IJobStatus.

6 files changed, 111 insertions, 78 deletions

diff --git a/core/org.eclipse.ptp.core/src/org/eclipse/ptp/rmsystem/IJobStatus.java b/core/org.eclipse.ptp.core/src/org/eclipse/ptp/rmsystem/IJobStatus.java
index 281d215d5..94424e08d 100644
--- a/core/org.eclipse.ptp.core/src/org/eclipse/ptp/rmsystem/IJobStatus.java
+++ b/core/org.eclipse.ptp.core/src/org/eclipse/ptp/rmsystem/IJobStatus.java
@@ -83,6 +83,12 @@ public interface IJobStatus {
 	public static String FAILED = "FAILED"; //$NON-NLS-1$
 
 	/**
+	 * Job was canceled/aborted by user before finishing. Jobs in this state
+	 * have completed execution and are considered "COMPLETED".
+	 */
+	public static String CANCELED = "CANCELED"; //$NON-NLS-1$
+
+	/**
 	 * Stdout and/or stderr have been copied to a user-determined location. Jobs
 	 * in this state have completed execution and are considered "COMPLETED".
 	 */
diff --git a/rms/org.eclipse.ptp.rm.jaxb.core/schema/JAXBResourceManagerConfigurations.exsd b/rms/org.eclipse.ptp.rm.jaxb.core/schema/JAXBResourceManagerConfigurations.exsd
index ae4c8b7b7..4ba1062ef 100644
--- a/rms/org.eclipse.ptp.rm.jaxb.core/schema/JAXBResourceManagerConfigurations.exsd
+++ b/rms/org.eclipse.ptp.rm.jaxb.core/schema/JAXBResourceManagerConfigurations.exsd
@@ -49,13 +49,6 @@
 
    <element name="JAXBResourceManagerConfiguration">
       <complexType>
-         <attribute name="id" type="string" use="required">
-            <annotation>
-               <documentation>
-                  ID of the resource manager that this configuration file is for.
-               </documentation>
-            </annotation>
-         </attribute>
          <attribute name="name" type="string" use="required">
             <annotation>
                <documentation>
diff --git a/rms/org.eclipse.ptp.rm.jaxb.core/src/org/eclipse/ptp/rm/jaxb/core/rm/JAXBResourceManagerControl.java b/rms/org.eclipse.ptp.rm.jaxb.core/src/org/eclipse/ptp/rm/jaxb/core/rm/JAXBResourceManagerControl.java
index 306319f68..1c176a501 100644
--- a/rms/org.eclipse.ptp.rm.jaxb.core/src/org/eclipse/ptp/rm/jaxb/core/rm/JAXBResourceManagerControl.java
+++ b/rms/org.eclipse.ptp.rm.jaxb.core/src/org/eclipse/ptp/rm/jaxb/core/rm/JAXBResourceManagerControl.java
@@ -164,9 +164,11 @@ public final class JAXBResourceManagerControl extends AbstractResourceManagerCon
 			p.setName(jobId);
 			rmVarMap.put(jobId, p);
 			doControlCommand(jobId, operation);
-			rmVarMap.remove(jobId);
 			if (TERMINATE_OPERATION.equals(operation)) {
-				jobStatusMap.removeJobStatus(jobId);
+				jobStatusMap.cancelAndRemove(jobId);
+				p.setValue(IJobStatus.CANCELED);
+			} else {
+				rmVarMap.remove(jobId);
 			}
 		} catch (CoreException ce) {
 			getResourceManager().setState(IResourceManager.ERROR_STATE);
@@ -226,23 +228,33 @@ public final class JAXBResourceManagerControl extends AbstractResourceManagerCon
 				status.setUpdateRequestTime(now);
 			}
 
-			PropertyType p = new PropertyType();
-			p.setVisible(false);
-			p.setName(jobId);
-			rmVarMap.put(jobId, p);
+			PropertyType p = (PropertyType) rmVarMap.get(jobId);
+			String state = IJobStatus.UNDETERMINED;
 
-			CommandType job = controlData.getGetJobStatus();
-			if (job == null) {
-				throw CoreExceptionUtils.newException(Messages.RMNoSuchCommandError + JOBSTATUS, null);
-			}
+			if (p != null) {
+				/*
+				 * premature termination
+				 */
+				state = String.valueOf(p.getValue());
+			} else {
+				state = status == null ? state : status.getStateDetail();
 
-			runCommand(jobId, job, false, true);
+				p = new PropertyType();
+				p.setVisible(false);
+				p.setName(jobId);
+				rmVarMap.put(jobId, p);
 
-			p = (PropertyType) rmVarMap.remove(jobId);
+				CommandType job = controlData.getGetJobStatus();
+				if (job == null) {
+					throw CoreExceptionUtils.newException(Messages.RMNoSuchCommandError + JOBSTATUS, null);
+				}
 
-			String state = IJobStatus.UNDETERMINED;
-			if (p != null) {
-				state = (String) p.getValue();
+				runCommand(jobId, job, false, true);
+
+				p = (PropertyType) rmVarMap.remove(jobId);
+				if (p != null) {
+					state = String.valueOf(p.getValue());
+				}
 			}
 
 			if (status == null) {
@@ -252,7 +264,7 @@ public final class JAXBResourceManagerControl extends AbstractResourceManagerCon
 				status.setState(state);
 			}
 
-			if (IJobStatus.COMPLETED.equals(state)) {
+			if (IJobStatus.COMPLETED.equals(status.getState())) {
 				/*
 				 * leave the status in the map in case there are further calls;
 				 * it will be pruned by the daemon
@@ -722,12 +734,6 @@ public final class JAXBResourceManagerControl extends AbstractResourceManagerCon
 		if (status != null) {
 			killed = status.cancel();
 		}
-		if (killed) {
-			/*
-			 * automatically unpins the id
-			 */
-			jobStatusMap.removeJobStatus(jobId);
-		}
 		return killed;
 	}
 
diff --git a/rms/org.eclipse.ptp.rm.jaxb.core/src/org/eclipse/ptp/rm/jaxb/core/runnable/JobStatusMap.java b/rms/org.eclipse.ptp.rm.jaxb.core/src/org/eclipse/ptp/rm/jaxb/core/runnable/JobStatusMap.java
index 7ab23d2bb..3186566e9 100644
--- a/rms/org.eclipse.ptp.rm.jaxb.core/src/org/eclipse/ptp/rm/jaxb/core/runnable/JobStatusMap.java
+++ b/rms/org.eclipse.ptp.rm.jaxb.core/src/org/eclipse/ptp/rm/jaxb/core/runnable/JobStatusMap.java
@@ -40,6 +40,21 @@ public class JobStatusMap extends Thread implements IJAXBNonNLSConstants {
 	}
 
 	/**
+	 * Synchronized cancel. External calls are premature and thus should not
+	 * block waiting for the remote files if any.
+	 * 
+	 * @param jobId
+	 *            either internal UUID or scheduler id for the job.
+	 */
+	public ICommandJobStatus cancelAndRemove(String jobId) {
+		ICommandJobStatus status = null;
+		synchronized (map) {
+			remove(jobId, false);
+		}
+		return status;
+	}
+
+	/**
 	 * 
 	 * @param jobId
 	 *            either internal UUID or scheduler id for the job.
@@ -64,21 +79,6 @@ public class JobStatusMap extends Thread implements IJAXBNonNLSConstants {
 	}
 
 	/**
-	 * Synchronized remove. External calls are premature and thus should not
-	 * block waiting for the remote files if any.
-	 * 
-	 * @param jobId
-	 *            either internal UUID or scheduler id for the job.
-	 */
-	public ICommandJobStatus removeJobStatus(String jobId) {
-		ICommandJobStatus status = null;
-		synchronized (map) {
-			status = remove(jobId, false);
-		}
-		return status;
-	}
-
-	/**
 	 * Thread daemon for cleanup on the map. Eliminates stray completed state
 	 * information, and also starts the stream proxies on jobs which have been
 	 * submitted to a scheduler and have become active.
diff --git a/rms/org.eclipse.ptp.rm.jaxb.core/src/org/eclipse/ptp/rm/jaxb/core/runnable/command/CommandJobStatus.java b/rms/org.eclipse.ptp.rm.jaxb.core/src/org/eclipse/ptp/rm/jaxb/core/runnable/command/CommandJobStatus.java
index 482a2e102..36e1d6442 100644
--- a/rms/org.eclipse.ptp.rm.jaxb.core/src/org/eclipse/ptp/rm/jaxb/core/runnable/command/CommandJobStatus.java
+++ b/rms/org.eclipse.ptp.rm.jaxb.core/src/org/eclipse/ptp/rm/jaxb/core/runnable/command/CommandJobStatus.java
@@ -34,6 +34,48 @@ import org.eclipse.ptp.rmsystem.IJobStatus;
  */
 public class CommandJobStatus implements ICommandJobStatus {
 
+	/**
+	 * Checks for file existence, then waits 3 seconds to compare file length.
+	 * If block is false, the listeners may be notified that the file is still
+	 * not ready; else the listeners will receive a ready = true notification
+	 * when the file does finally stabilize, provided this occurs within the
+	 * block parameter (seconds).
+	 * 
+	 * @author arossi
+	 */
+	private class FileReadyChecker extends Thread {
+		private boolean ready;
+		private int block;
+		private String path;
+
+		@Override
+		public void run() {
+			ready = false;
+			long timeout = block * 1000;
+			RemoteServicesDelegate d = control.getRemoteServicesDelegate();
+			long start = System.currentTimeMillis();
+			while (!ready) {
+				try {
+					ready = FileUtils.isStable(d.getRemoteFileManager(), path, 3, new NullProgressMonitor());
+				} catch (Throwable t) {
+					JAXBCorePlugin.log(t);
+				}
+
+				if (System.currentTimeMillis() - start >= timeout) {
+					break;
+				}
+
+				synchronized (this) {
+					try {
+						wait(IJAXBNonNLSConstants.STANDARD_WAIT);
+					} catch (InterruptedException ignored) {
+					}
+				}
+			}
+		}
+
+	}
+
 	private final String rmUniqueName;
 	private final IJAXBResourceManagerControl control;
 
@@ -239,8 +281,8 @@ public class CommandJobStatus implements ICommandJobStatus {
 			return;
 		}
 
-		Thread tout = null;
-		Thread terr = null;
+		FileReadyChecker tout = null;
+		FileReadyChecker terr = null;
 
 		if (remoteOutputPath != null) {
 			tout = checkForReady(remoteOutputPath, blockForSecs);
@@ -251,6 +293,11 @@ public class CommandJobStatus implements ICommandJobStatus {
 			terr = checkForReady(remoteErrorPath, blockForSecs);
 		}
 
+		if (tout == null && terr == null) {
+			fFilesChecked = true;
+			return;
+		}
+
 		if (tout != null) {
 			try {
 				tout.join();
@@ -264,7 +311,11 @@ public class CommandJobStatus implements ICommandJobStatus {
 			} catch (InterruptedException ignored) {
 			}
 		}
-		setState(IJobStatus.JOB_OUTERR_READY);
+
+		if ((tout == null || tout.ready) && (terr == null || terr.ready)) {
+			setState(IJobStatus.JOB_OUTERR_READY);
+		}
+
 		fFilesChecked = true;
 	}
 
@@ -344,6 +395,9 @@ public class CommandJobStatus implements ICommandJobStatus {
 		} else if (FAILED.equals(state)) {
 			this.state = COMPLETED;
 			stateDetail = FAILED;
+		} else if (CANCELED.equals(state)) {
+			this.state = COMPLETED;
+			stateDetail = CANCELED;
 		} else if (JOB_OUTERR_READY.equals(state)) {
 			this.state = COMPLETED;
 			stateDetail = JOB_OUTERR_READY;
@@ -416,34 +470,10 @@ public class CommandJobStatus implements ICommandJobStatus {
 	 * @param blockInSeconds
 	 * @return thread running the check
 	 */
-	private Thread checkForReady(final String path, final int block) {
-		Thread t = new Thread() {
-			@Override
-			public void run() {
-				boolean ready = false;
-				long timeout = block * 1000;
-				RemoteServicesDelegate d = control.getRemoteServicesDelegate();
-				long start = System.currentTimeMillis();
-				while (!ready) {
-					try {
-						ready = FileUtils.isStable(d.getRemoteFileManager(), path, 3, new NullProgressMonitor());
-					} catch (Throwable t) {
-						JAXBCorePlugin.log(t);
-					}
-
-					if (System.currentTimeMillis() - start >= timeout) {
-						break;
-					}
-
-					synchronized (this) {
-						try {
-							wait(IJAXBNonNLSConstants.STANDARD_WAIT);
-						} catch (InterruptedException ignored) {
-						}
-					}
-				}
-			}
-		};
+	private FileReadyChecker checkForReady(final String path, final int block) {
+		FileReadyChecker t = new FileReadyChecker();
+		t.block = block;
+		t.path = path;
 		t.start();
 		return t;
 	}
diff --git a/rms/org.eclipse.ptp.rm.jaxb.core/src/org/eclipse/ptp/rm/jaxb/core/utils/JobIdPinTable.java b/rms/org.eclipse.ptp.rm.jaxb.core/src/org/eclipse/ptp/rm/jaxb/core/utils/JobIdPinTable.java
index f23ba8849..bb3a25ec8 100644
--- a/rms/org.eclipse.ptp.rm.jaxb.core/src/org/eclipse/ptp/rm/jaxb/core/utils/JobIdPinTable.java
+++ b/rms/org.eclipse.ptp.rm.jaxb.core/src/org/eclipse/ptp/rm/jaxb/core/utils/JobIdPinTable.java
@@ -37,9 +37,7 @@ public class JobIdPinTable implements IJAXBNonNLSConstants {
 				} catch (InterruptedException ignored) {
 				}
 			}
-			if (map.containsKey(jobId)) {
-				map.put(jobId, Thread.currentThread());
-			}
+			map.put(jobId, Thread.currentThread());
 		}
 	}