From 5c830c3c9c91f7584e673f3175e3efd7ec525e29 Mon Sep 17 00:00:00 2001 From: Nate Jensen Date: Wed, 3 Apr 2013 14:33:24 -0500 Subject: [PATCH] Issue #1855 fix python threading in GFE procedures and smart tools Change-Id: I6420338991c1961ce63ff944b516925f83a12994 Former-commit-id: fe24e34163bc32aed3cc29d5a17af051e13b99e1 [formerly c4549e0279a3026adc2f7b40bb272fc4f59acf59] Former-commit-id: e3462292abf2c9326d2e89ea08f5a40585005b1c --- .../viz/gfe/procedures/ProcedureJob.java | 53 +++++++++++-------- .../gfe/smarttool/script/SmartToolJob.java | 27 +++------- 2 files changed, 38 insertions(+), 42 deletions(-) diff --git a/cave/com.raytheon.viz.gfe/src/com/raytheon/viz/gfe/procedures/ProcedureJob.java b/cave/com.raytheon.viz.gfe/src/com/raytheon/viz/gfe/procedures/ProcedureJob.java index bd6ab0f0f4..21c8f4523e 100644 --- a/cave/com.raytheon.viz.gfe/src/com/raytheon/viz/gfe/procedures/ProcedureJob.java +++ b/cave/com.raytheon.viz.gfe/src/com/raytheon/viz/gfe/procedures/ProcedureJob.java @@ -58,6 +58,8 @@ import com.raytheon.viz.gfe.jobs.AsyncProgressJob; * Oct 8, 2009 njensen Initial creation * Jan 8, 2013 1486 dgilling Support changes to BaseGfePyController. * Jan 18, 2013 1509 njensen Garbage collect after running procedure + * Apr 03, 2013 1855 njensen Never dispose interpreters until shutdown and + * reuse interpreter if called from procedure * * * @@ -69,12 +71,7 @@ public class ProcedureJob extends AbstractQueueJob { /** * Maximum number of jobs to keep for a given Data Manager. */ - private final static int maxJobs = 3; - - /** - * Amount of time to keep inactive jobs servers around. - */ - private final static long expireTime = 5L * 60L * 1000L; + private final static int maxJobs = 4; /** * Index of job with the queue. Will break code if not zero. @@ -121,14 +118,12 @@ public class ProcedureJob extends AbstractQueueJob { */ @Override protected IStatus run(IProgressMonitor monitor) { - long starTime = System.currentTimeMillis(); - boolean expireJob = instanceMap.get(dataMgr).get(QUEUE_JOB_INDEX) != this; try { python = ProcedureFactory.buildController(dataMgr); } catch (JepException e) { ProcedureJob.removeJob(dataMgr, this); return new Status(IStatus.ERROR, StatusConstants.PLUGIN_ID, - "Error initializing guidance python", e); + "Error initializing procedure python", e); } try { @@ -151,15 +146,10 @@ public class ProcedureJob extends AbstractQueueJob { if (request != null) { request.requestComplete(null); } - } else if (expireJob - && ((instanceMap.get(dataMgr).size() > maxJobs) || (System - .currentTimeMillis() - starTime) > expireTime)) { - ProcedureJob.removeJob(dataMgr, this); - break; } } catch (Throwable t) { statusHandler.handle(Priority.PROBLEM, - "Error running tool ", t); + "Error running procedure ", t); if (request != null) { request.requestComplete(t); } @@ -229,6 +219,7 @@ public class ProcedureJob extends AbstractQueueJob { instanceMap = new HashMap>(); } + Thread currentThread = Thread.currentThread(); List jobList = instanceMap.get(dataMgr); if (jobList == null) { jobList = new ArrayList(); @@ -241,17 +232,26 @@ public class ProcedureJob extends AbstractQueueJob { job.schedule(); } boolean jobAvailable = false; + ProcedureJob alreadyOnThread = null; for (ProcedureJob job : jobList) { - if (job.request == null) { + Thread jobThread = job.getThread(); + if (currentThread == jobThread) { + // this occurs when a running procedure uses + // SmartScript.callProcedure() + // for efficiency we want to just stay on this thread + alreadyOnThread = job; + jobAvailable = true; + break; + } else if (job.request == null) { jobAvailable = true; break; } } - // All jobs for data manager are busy add another. - // To mimic AWIPS I allow any number of jobs. - // The check in the run will reduce the number to maxJobs. - if (jobAvailable == false) { + // All jobs for data manager are busy, add another if we haven't + // reached the limit. + if (alreadyOnThread == null && !jobAvailable + && jobList.size() < maxJobs) { ProcedureJob job = new ProcedureJob(dataMgr); job.setSystem(true); jobList.add(job); @@ -261,7 +261,18 @@ public class ProcedureJob extends AbstractQueueJob { jobAvailable = true; } - jobList.get(QUEUE_JOB_INDEX).enqueue(request); + if (alreadyOnThread != null) { + try { + alreadyOnThread.processRequest(request); + request.requestComplete(null); + } catch (Throwable t) { + statusHandler.handle(Priority.PROBLEM, + "Error running procedure ", t); + request.requestComplete(t); + } + } else { + jobList.get(QUEUE_JOB_INDEX).enqueue(request); + } return jobAvailable; } diff --git a/cave/com.raytheon.viz.gfe/src/com/raytheon/viz/gfe/smarttool/script/SmartToolJob.java b/cave/com.raytheon.viz.gfe/src/com/raytheon/viz/gfe/smarttool/script/SmartToolJob.java index c154f9439b..2165c3dfa4 100644 --- a/cave/com.raytheon.viz.gfe/src/com/raytheon/viz/gfe/smarttool/script/SmartToolJob.java +++ b/cave/com.raytheon.viz.gfe/src/com/raytheon/viz/gfe/smarttool/script/SmartToolJob.java @@ -54,6 +54,7 @@ import com.raytheon.viz.gfe.smarttool.Tool; * ------------ ---------- ----------- -------------------------- * Jan 19, 2010 njensen Initial creation * Jan 18, 2013 1509 njensen Garbage collect after running tool + * Apr 03, 2013 1855 njensen Never dispose interpreters until shutdown * * * @@ -68,11 +69,6 @@ public class SmartToolJob extends AbstractQueueJob { */ private final static int maxJobs = 3; - /** - * Amount of time to keep inactive jobs servers around. - */ - private final static long expireTime = 5L * 60L * 1000L; - /** * Index of job with the queue. Will break code if not zero. */ @@ -114,14 +110,12 @@ public class SmartToolJob extends AbstractQueueJob { @Override protected IStatus run(IProgressMonitor monitor) { SmartToolController python = null; - long starTime = System.currentTimeMillis(); - boolean expireJob = instanceMap.get(dataMgr).get(QUEUE_JOB_INDEX) != this; try { python = SmartToolFactory.buildController(dataMgr); } catch (JepException e) { SmartToolJob.removeJob(dataMgr, this); return new Status(IStatus.ERROR, StatusConstants.PLUGIN_ID, - "Error initializing guidance python", e); + "Error initializing smart tool python", e); } try { @@ -139,7 +133,6 @@ public class SmartToolJob extends AbstractQueueJob { synchronized (this) { if (request != null) { - starTime = System.currentTimeMillis(); python.processFileUpdates(); EditAction ea = request.getPreview() .getEditAction(); @@ -170,11 +163,6 @@ public class SmartToolJob extends AbstractQueueJob { req = request; request = null; } - } else if (expireJob - && ((instanceMap.get(dataMgr).size() > maxJobs) || (System - .currentTimeMillis() - starTime) > expireTime)) { - SmartToolJob.removeJob(dataMgr, this); - break; } } } catch (InterruptedException e) { @@ -201,10 +189,8 @@ public class SmartToolJob extends AbstractQueueJob { req = null; } } - System.err.println("SmartToolJob exit loop: " - + monitor.isCanceled()); } finally { - System.err.println("shutdown instance of SmartToolJob"); + System.err.println("Shutdown instance of SmartToolJob"); if (python != null) { python.dispose(); python = null; @@ -277,10 +263,9 @@ public class SmartToolJob extends AbstractQueueJob { } } - // All jobs for data manager are busy add another. - // To mimic AWIPS I allow any number of jobs. - // The check in the run will reduce the number to maxJobs. - if (jobAvailable == false) { + // All jobs for data manager are busy, add another if we haven't reached + // the limit + if (!jobAvailable && jobList.size() < maxJobs) { SmartToolJob job = new SmartToolJob(dataMgr); job.setSystem(true); jobList.add(job);