Fix multi-threading, initialize V8 only once

Makefile.frag

@@ -3,6 +3,11 @@ ifneq (,$(realpath $(EXTENSION_DIR)/json.so))
 PHP_TEST_SHARED_EXTENSIONS+=-d extension=$(EXTENSION_DIR)/json.so
 endif
 
+# add pthreads extension, if available
+ifneq (,$(realpath $(EXTENSION_DIR)/pthreads.so))
+PHP_TEST_SHARED_EXTENSIONS+=-d extension=$(EXTENSION_DIR)/pthreads.so
+endif
+
 testv8: all
 	$(PHP_EXECUTABLE) -n -d extension_dir=./modules -d extension=v8js.so test.php
 

php_v8js_macros.h

@@ -111,10 +111,8 @@ void v8js_register_accessors(std::vector<v8js_accessor_ctx*> *accessor_list, v8:
 
 /* Module globals */
 ZEND_BEGIN_MODULE_GLOBALS(v8js)
+  // Thread-local cache whether V8 has been initialized so far
   int v8_initialized;
-#if !defined(_WIN32) && PHP_V8_API_VERSION >= 3029036
-  v8::Platform *v8_platform;
-#endif
   HashTable *extensions;
 
   /* Ini globals */
@@ -142,6 +140,32 @@ ZEND_EXTERN_MODULE_GLOBALS(v8js)
 # define V8JSG(v) (v8js_globals.v)
 #endif
 
+/*
+ *  Process-wide globals
+ *
+ * The zend_v8js_globals structure declared above is created once per thread
+ * (in a ZTS environment).  If a multi-threaded PHP process uses V8 there is
+ * some stuff shared among all of these threads
+ *
+ *  - whether V8 has been initialized at all
+ *  - the V8 backend platform
+ *
+ * In a ZTS-enabled environment access to all of these variables must happen
+ * while holding a mutex lock.
+ */
+struct _v8js_process_globals {
+#ifdef ZTS
+	int v8_initialized;
+	std::mutex lock;
+#endif
+
+#if !defined(_WIN32) && PHP_V8_API_VERSION >= 3029036
+	v8::Platform *v8_platform;
+#endif
+};
+
+extern struct _v8js_process_globals v8js_process_globals;
+
 /* Register builtin methods into passed object */
 void v8js_register_methods(v8::Handle<v8::ObjectTemplate>, v8js_ctx *c);
 

tests/pthreads_001.phpt

@@ -0,0 +1,65 @@
+--TEST--
+Test V8::executeString() : Pthreads test #1
+--SKIPIF--
+<?php
+require_once(dirname(__FILE__) . '/skipif.inc');
+if(!class_exists('Thread')) {
+    die('skip pthreads extension required');
+}
+?>
+--FILE--
+<?php
+
+class Workhorse extends Thread
+{
+    protected $val;
+
+    public function __construct($val)
+    {
+        $this->val = $val;
+    }
+
+    public function run()
+    {
+        $v8 = new V8Js();
+        $v8->val = $this->val;
+        $v8->sync_var_dump = function($value) {
+            $this->synchronized(function($thread) use ($value) {
+                    while(!$thread->readyToPrint) {
+                        $thread->wait();
+                    }
+                    var_dump($value);
+                    $thread->notify();
+                }, $this);
+        };
+
+        $v8->executeString('PHP.sync_var_dump(PHP.val);');
+    }
+}
+
+$foo = new Workhorse('foo');
+$bar = new Workhorse('bar');
+
+$foo->start();
+$bar->start();
+
+$bar->synchronized(function($thread) {
+    $thread->readyToPrint = true;
+    $thread->notify();
+    $thread->wait();
+}, $bar);
+
+$foo->synchronized(function($thread) {
+    $thread->readyToPrint = true;
+    $thread->notify();
+    $thread->wait();
+}, $foo);
+
+$foo->join();
+$bar->join();
+?>
+===EOF===
+--EXPECT--
+string(3) "bar"
+string(3) "foo"
+===EOF===

v8js.cc

@@ -29,6 +29,7 @@ extern "C" {
 #include "v8js_v8object_class.h"
 
 ZEND_DECLARE_MODULE_GLOBALS(v8js)
+struct _v8js_process_globals v8js_process_globals;
 
 /* {{{ INI Settings */
 
@@ -118,6 +119,16 @@ PHP_MINIT_FUNCTION(v8js)
 static PHP_MSHUTDOWN_FUNCTION(v8js)
 {
 	UNREGISTER_INI_ENTRIES();
+
+	if(v8js_process_globals.v8_initialized) {
+		v8::V8::Dispose();
+#if !defined(_WIN32) && PHP_V8_API_VERSION >= 3029036
+		v8::V8::ShutdownPlatform();
+		// @fixme call virtual destructor somehow
+		//delete v8js_process_globals.v8_platform;
+#endif
+	}
+
 	return SUCCESS;
 }
 /* }}} */
@@ -202,14 +213,6 @@ static PHP_GSHUTDOWN_FUNCTION(v8js)
 	v8js_globals->timer_stack.~deque();
 	v8js_globals->timer_mutex.~mutex();
 #endif
-
-	if (v8js_globals->v8_initialized) {
-		v8::V8::Dispose();
-#if !defined(_WIN32) && PHP_V8_API_VERSION >= 3029036
-		v8::V8::ShutdownPlatform();
-		delete v8js_globals->v8_platform;
-#endif
-	}
 }
 /* }}} */
 

v8js_v8.cc

@@ -37,14 +37,27 @@ extern "C" {
 
 void v8js_v8_init(TSRMLS_D) /* {{{ */
 {
-	/* Run only once */
+	/* Run only once; thread-local test first */
 	if (V8JSG(v8_initialized)) {
 		return;
 	}
 
+	/* Set thread-local flag, that V8 was initialized. */
+	V8JSG(v8_initialized) = 1;
+
+#ifdef ZTS
+	v8js_process_globals.lock.lock();
+
+	if(v8js_process_globals.v8_initialized) {
+		/* V8 already has been initialized by another thread */
+		v8js_process_globals.lock.unlock();
+		return;
+	}
+#endif
+
 #if !defined(_WIN32) && PHP_V8_API_VERSION >= 3029036
-	V8JSG(v8_platform) = v8::platform::CreateDefaultPlatform();
+	v8js_process_globals.v8_platform = v8::platform::CreateDefaultPlatform();
-	v8::V8::InitializePlatform(V8JSG(v8_platform));
+	v8::V8::InitializePlatform(v8js_process_globals.v8_platform);
 #endif
 
 	/* Set V8 command line flags (must be done before V8::Initialize()!) */
@@ -55,8 +68,10 @@ void v8js_v8_init(TSRMLS_D) /* {{{ */
 	/* Initialize V8 */
 	v8::V8::Initialize();
 
-	/* Run only once */
+#ifdef ZTS
-	V8JSG(v8_initialized) = 1;
+	v8js_process_globals.v8_initialized = 1;
+	v8js_process_globals.lock.unlock();
+#endif
 }
 /* }}} */