Fix exec_js_process process caching (#701)

* Fix exec_js_process process caching

The existing logic doesn't properly cache processes, leading to a new process being created for every invocation of `exec_js_process` (eventually leading to an error due to too many processes with many calls to execjs). This commit fixes this logic and adds tests for this.

Author: ThomasHickman

cwltool/sandboxjs.py

@@ -142,18 +142,27 @@ def exec_js_process(js_text, timeout=None, js_console=False, context=None, force
 
     created_new_process = False
 
-    if (context is None and localdata.procs.get(js_engine) is None) \
-            or (context is not None and localdata.procs.get((js_engine, context)) is None) \
-            or localdata.procs[js_engine].poll() is not None \
+    if context is None:
+        nodejs = localdata.procs.get(js_engine)
+    else:
+        nodejs = localdata.procs.get((js_engine, context))
+
+    if nodejs is None \
+            or nodejs.poll() is not None \
             or onWindows():
         res = resource_stream(__name__, js_engine)
         js_engine_code = res.read().decode('utf-8')
 
         created_new_process = True
 
-        localdata.procs[js_engine] = new_js_proc(js_engine_code, force_docker_pull=force_docker_pull)
+        new_proc = new_js_proc(js_engine_code, force_docker_pull=force_docker_pull)
 
-    nodejs = localdata.procs[js_engine]
+        if context is None:
+            localdata.procs[js_engine] = new_proc
+            nodejs = new_proc
+        else:
+            localdata.procs[(js_engine, context)] = new_proc
+            nodejs = new_proc
 
     killed = []
 

tests/test_js_sandbox.py

@@ -2,20 +2,25 @@
 
 import unittest
 
-from mock import Mock
+from mock import Mock, patch
+from mock import MagicMock
 
 import cwltool
 import cwltool.factory
 # we should modify the subprocess imported from cwltool.sandboxjs
 from cwltool.sandboxjs import (check_js_threshold_version,
-                               subprocess)
+                               subprocess,
+                               exec_js_process)
+import cwltool.sandboxjs
+from cwltool.utils import onWindows
 from .util import get_data
+import pytest
 
 
 class Javascript_Sanity_Checks(unittest.TestCase):
 
     def setUp(self):
-        self.check_output = subprocess.check_output 
+        self.check_output = subprocess.check_output
 
     def tearDown(self):
         subprocess.check_output = self.check_output
@@ -37,7 +42,7 @@ def test_node_version(self):
         self.assertEquals(check_js_threshold_version('node'), True)
 
     def test_is_javascript_installed(self):
-            pass
+        pass
 
 
 class TestValueFrom(unittest.TestCase):
@@ -46,3 +51,14 @@ def test_value_from_two_concatenated_expressions(self):
         f = cwltool.factory.Factory()
         echo = f.make(get_data("tests/wf/vf-concat.cwl"))
         self.assertEqual(echo(), {u"out": u"a sting\n"})
+
+
+class ExecJsProcessTest(unittest.TestCase):
+    @pytest.mark.skipif(onWindows(),
+                        reason="Caching processes for windows is not supported.")
+    def test_caches_js_processes(self):
+        exec_js_process("7", context="{}")
+
+        with patch("cwltool.sandboxjs.new_js_proc", new=Mock(wraps=cwltool.sandboxjs.new_js_proc)) as mock:
+            exec_js_process("7", context="{}")
+            mock.assert_not_called()