Disallow multiple threads using the same Context, even at different times.

Author: PetterS

module.c

@@ -211,12 +211,20 @@ static PyObject *quickjs_to_python(ContextData *context_obj, JSValue value) {
 		JSValue exception = JS_GetException(context);
 		JSValue error_string = JS_ToString(context, exception);
 		const char *cstring = JS_ToCString(context, error_string);
-		if (strstr(cstring, "stack overflow") != NULL) {
-			PyErr_Format(StackOverflow, "%s", cstring);
+		if (cstring != NULL) {
+			if (strstr(cstring, "stack overflow") != NULL) {
+				PyErr_Format(StackOverflow, "%s", cstring);
+			} else {
+				PyErr_Format(JSException, "%s", cstring);
+			}
+			JS_FreeCString(context, cstring);
 		} else {
-			PyErr_Format(JSException, "%s", cstring);
+			// This has been observed to happen when different threads have used the same QuickJS
+			// runtime, but not at the same time.
+			// Could potentially be another problem though, since JS_ToCString may return NULL.
+			PyErr_Format(JSException,
+			             "(Failed obtaining QuickJS error string. Concurrency issue?)");
 		}
-		JS_FreeCString(context, cstring);
 		JS_FreeValue(context, error_string);
 		JS_FreeValue(context, exception);
 	} else if (tag == JS_TAG_FLOAT64) {

quickjs/__init__.py

@@ -1,5 +1,7 @@
+import concurrent.futures
 import json
 import threading
+from typing import Tuple
 
 import _quickjs
 
@@ -15,15 +17,33 @@ def test():
 
 
 class Function:
-    def __init__(self, name: str, code: str) -> None:
-        self._context = Context()
-        self._context.eval(code)
-        self._f = self._context.get(name)
+    # There are unit tests demonstrating that we are crashing if different threads are accessing the
+    # same runtime, even if it is not at the same time. So we run everything on the same thread in
+    # order to prevent this.
+    _threadpool = concurrent.futures.ThreadPoolExecutor(max_workers=1)
+    
+    def __init__(self, name: str, code: str, *, own_executor=False) -> None:
+        """
+        Arguments:
+            name: The name of the function in the provided code that will be executed.
+            code: The source code of the function and possibly helper functions, classes, global
+                  variables etc.
+            own_executor: Create an executor specifically for this function. The default is False in
+                          order to save system resources if a large number of functions are created.
+        """
+        if own_executor:
+            self._threadpool = concurrent.futures.ThreadPoolExecutor(max_workers=1)
         self._lock = threading.Lock()
 
+        future = self._threadpool.submit(self._compile, name, code)
+        concurrent.futures.wait([future])
+        self._context, self._f = future.result()
+
     def __call__(self, *args, run_gc=True):
         with self._lock:
-            return self._call(*args, run_gc=run_gc)
+            future = self._threadpool.submit(self._call, *args, run_gc=run_gc)
+            concurrent.futures.wait([future])
+            return future.result()
 
     def set_memory_limit(self, limit):
         with self._lock:
@@ -49,6 +69,12 @@ def gc(self):
         with self._lock:
             self._context.gc()
 
+    def _compile(self, name: str, code: str) -> Tuple[Context, Object]:
+        context = Context()
+        context.eval(code)
+        f = context.get(name)
+        return context, f
+
     def _call(self, *args, run_gc=True):
         def convert_arg(arg):
             if isinstance(arg, (type(None), str, bool, float, int)):

test_quickjs.py

@@ -1,3 +1,4 @@
+import concurrent.futures
 import json
 import unittest
 
@@ -340,3 +341,53 @@ def test_unicode(self):
         for x in ["äpple", "≤≥", "☺"]:
             self.assertEqual(identity(x), x)
             self.assertEqual(context.eval('(function(){ return "' + x + '";})()'), x)
+
+
+class Threads(unittest.TestCase):
+    def setUp(self):
+        self.context = quickjs.Context()
+        self.executor = concurrent.futures.ThreadPoolExecutor()
+
+    def tearDown(self):
+        self.executor.shutdown()
+
+    def test_concurrent(self):
+        """Demonstrates that the execution will crash unless the function executes on the same
+           thread every time.
+
+           If the executor in Function is not present, this test will fail.
+        """
+        data = list(range(1000))
+        jssum = quickjs.Function(
+            "sum", """
+                function sum(data) {
+                    return data.reduce((a, b) => a + b, 0)
+                }
+            """)
+
+        futures = [self.executor.submit(jssum, data) for _ in range(10)]
+        expected = sum(data)
+        for future in concurrent.futures.as_completed(futures):
+            self.assertEqual(future.result(), expected)
+
+    def test_concurrent_own_executor(self):
+        data = list(range(1000))
+        jssum1 = quickjs.Function("sum",
+                                  """
+                                    function sum(data) {
+                                        return data.reduce((a, b) => a + b, 0)
+                                    }
+                                  """,
+                                  own_executor=True)
+        jssum2 = quickjs.Function("sum",
+                                  """
+                                    function sum(data) {
+                                        return data.reduce((a, b) => a + b, 0)
+                                    }
+                                  """,
+                                  own_executor=True)
+
+        futures = [self.executor.submit(f, data) for _ in range(10) for f in (jssum1, jssum2)]
+        expected = sum(data)
+        for future in concurrent.futures.as_completed(futures):
+            self.assertEqual(future.result(), expected)