Merge pull request #87 from datastax/115

115 - Fix: Duplicate UUIDs generated in forked processes

Author: mpenick

CHANGELOG.md

@@ -1,3 +1,11 @@
+# 1.2.2
+
+Bug Fixes:
+
+* [PHP-88] \Cassandra\Timestamp::toDateTime segfault with PHP7
+* [PHP-112] Freeing a null future as result of a failure in \Cassandra\DefaultSession::executeAsync()
+* [PHP-115] \Cassandra\UUID returning duplicate UUIDs
+
 # 1.2.1
 
 Bug Fixes:

ext/package.xml

@@ -14,23 +14,25 @@ protocol and Cassandra Query Language v3.
     <email>[email protected]</email>
     <active>yes</active>
   </lead>
-  <date>2016-07-28</date>
-  <time>08:37:25</time>
+  <date>2016-08-08</date>
+  <time>11:25:32</time>
   <version>
-    <release>1.2.1</release>
-    <api>1.2.1</api>
+    <release>1.2.2</release>
+    <api>1.2.2</api>
   </version>
   <stability>
     <release>stable</release>
     <api>stable</api>
   </stability>
   <license uri="http://www.apache.org/licenses/LICENSE-2.0">Apache License 2.0</license>
   <notes>
-# 1.2.1
+# 1.2.2
 
 Bug Fixes:
 
-* [PHP-113] pecl install of 1.2.0 fails because sourcecode is missing FutureRows.h
+* [PHP-88] \Cassandra\Timestamp::toDateTime segfault with PHP7
+* [PHP-112] Freeing a null future as result of a failure in \Cassandra\DefaultSession::executeAsync()
+* [PHP-115] \Cassandra\UUID returning duplicate UUIDs
   </notes>
   <contents>
     <dir name="/">

ext/php_cassandra.c

@@ -273,7 +273,8 @@ static PHP_GINIT_FUNCTION(cassandra)
 {
   uv_once(&log_once, php_cassandra_log_initialize);
 
-  cassandra_globals->uuid_gen            = cass_uuid_gen_new();
+  cassandra_globals->uuid_gen            = NULL;
+  cassandra_globals->uuid_gen_pid        = 0;
   cassandra_globals->persistent_clusters = 0;
   cassandra_globals->persistent_sessions = 0;
   PHP5TO7_ZVAL_UNDEF(cassandra_globals->type_varchar);
@@ -297,7 +298,9 @@ static PHP_GINIT_FUNCTION(cassandra)
 
 static PHP_GSHUTDOWN_FUNCTION(cassandra)
 {
-  cass_uuid_gen_free(cassandra_globals->uuid_gen);
+  if (cassandra_globals->uuid_gen) {
+    cass_uuid_gen_free(cassandra_globals->uuid_gen);
+  }
   php_cassandra_log_cleanup();
 }
 

ext/php_cassandra.h

@@ -38,6 +38,19 @@
 #include <Zend/zend_exceptions.h>
 #include <Zend/zend_interfaces.h>
 
+#ifdef HAVE_SYS_TYPES_H
+#include <sys/types.h>
+#endif
+
+#ifdef HAVE_UNISTD_H
+#include <unistd.h>
+#endif
+
+#ifdef PHP_WIN32
+typedef int pid_t;
+#include <process.h>
+#endif
+
 #if PHP_VERSION_ID < 50304
 #  error PHP 5.3.4 or later is required in order to build the driver
 #endif
@@ -478,6 +491,7 @@ PHP_MINFO_FUNCTION(cassandra);
 
 ZEND_BEGIN_MODULE_GLOBALS(cassandra)
   CassUuidGen          *uuid_gen;
+  pid_t                 uuid_gen_pid;
   unsigned int          persistent_clusters;
   unsigned int          persistent_sessions;
   php5to7_zval          type_varchar;

ext/util/uuid_gen.c

@@ -20,20 +20,40 @@
 
 ZEND_EXTERN_MODULE_GLOBALS(cassandra)
 
+static CassUuidGen* get_uuid_gen(TSRMLS_D) {
+  /* Create a new uuid generator if our PID has changed. This prevents the same
+   * UUIDs from being generated in forked processes.
+   */
+  if (CASSANDRA_G(uuid_gen_pid) != getpid()) {
+    if (CASSANDRA_G(uuid_gen)) {
+      cass_uuid_gen_free(CASSANDRA_G(uuid_gen));
+    }
+    CASSANDRA_G(uuid_gen) = cass_uuid_gen_new();
+    CASSANDRA_G(uuid_gen_pid) = getpid();
+  }
+  return CASSANDRA_G(uuid_gen);
+}
+
 void
 php_cassandra_uuid_generate_random(CassUuid *out TSRMLS_DC)
 {
-  cass_uuid_gen_random(CASSANDRA_G(uuid_gen), out);
+  CassUuidGen* uuid_gen = get_uuid_gen(TSRMLS_C);
+  if (!uuid_gen) return;
+  cass_uuid_gen_random(uuid_gen, out);
 }
 
 void
 php_cassandra_uuid_generate_time(CassUuid *out TSRMLS_DC)
 {
-  cass_uuid_gen_time(CASSANDRA_G(uuid_gen), out);
+  CassUuidGen* uuid_gen = get_uuid_gen(TSRMLS_C);
+  if (!uuid_gen) return;
+  cass_uuid_gen_time(uuid_gen, out);
 }
 
 void
 php_cassandra_uuid_generate_from_time(long timestamp, CassUuid *out TSRMLS_DC)
 {
-  cass_uuid_gen_from_time(CASSANDRA_G(uuid_gen), (cass_uint64_t) timestamp, out);
+  CassUuidGen* uuid_gen = get_uuid_gen(TSRMLS_C);
+  if (!uuid_gen) return;
+  cass_uuid_gen_from_time(uuid_gen, (cass_uint64_t) timestamp, out);
 }

ext/version.h

@@ -5,9 +5,9 @@
 #define PHP_CASSANDRA_NAME         "cassandra"
 #define PHP_CASSANDRA_MAJOR        1
 #define PHP_CASSANDRA_MINOR        2
-#define PHP_CASSANDRA_RELEASE      1
+#define PHP_CASSANDRA_RELEASE      2
 #define PHP_CASSANDRA_STABILITY    "stable"
-#define PHP_CASSANDRA_VERSION      "1.2.1"
-#define PHP_CASSANDRA_VERSION_FULL "1.2.1"
+#define PHP_CASSANDRA_VERSION      "1.2.2"
+#define PHP_CASSANDRA_VERSION_FULL "1.2.2"
 
 #endif /* PHP_CASSANDRA_VERSION_H */

tests/unit/Cassandra/UuidTest.php

@@ -61,4 +61,71 @@ public function notEqualTypes()
             array(new Uuid('2a5072fa-7da4-4ccd-a9b4-f017a3872304'), new Uuid('3b5072fa-7da4-4ccd-a9b4-f017a3872304')),
         );
     }
+
+    /**
+     * Ensure UUIDs are unique for fork/child processes
+     *
+     * This test will ensure that the PHP driver is producing unique UUIDs for
+     * all child processes that get created during fork() operations in web
+     * servers (e.g. Apache, nginx, ...etc).
+     *
+     * @test
+     * @ticket PHP-115
+     */
+    public function testUniqueInChild() {
+        if (!function_exists("pcntl_fork")) {
+            $this->markTestSkipped("Unable to Execute testUniqueInChild Unit Test: pcntl_fork() does not exists");
+        } else {
+            // Create a PHP script to call within a PHPUnit test (exit call fails test)
+            $script = <<<EOF
+<?php
+// Get and open the file for appending child UUIDs
+\$uuidsFilename = \$_SERVER['argv'][1];
+\$numberOfForks = \$_SERVER['argv'][2];
+
+// Create requested children process; create UUIDs and append to a file
+\$children = array();
+foreach (range(1, \$numberOfForks) as \$i) {
+    // Create the child process
+    \$pid = pcntl_fork();
+
+    // Ensure the child process was create successfully
+    if (\$pid < 0) {
+        die("Unable to Create Fork: Unique UUID test cannot complete");
+    } else if (\$pid === 0) {
+        // Create a UUID and add it to the file
+        \$uuid = new \Cassandra\Uuid();
+        file_put_contents(\$uuidsFilename, \$uuid->uuid() . PHP_EOL, FILE_APPEND);
+
+        // Terminate child process
+        exit(0);
+    } else {
+        // Parent process: Add the process ID to force waiting on children
+        \$children[] = \$pid;
+    }
+}
+
+// Wait on each child process to finish
+foreach (\$children as \$pid) {
+    pcntl_waitpid(\$pid, \$status);
+}
+?>
+EOF;
+            $numProcesses = 64;
+
+            // Execute the PHP script passing in the filename for the UUIDs to be stored
+            $uuidsFilename = tempnam(sys_get_temp_dir(), "uuid");
+            $scriptFilename = tempnam(sys_get_temp_dir(), "uuid");
+            file_put_contents($scriptFilename, $script, FILE_APPEND);
+            exec(PHP_BINARY . " {$scriptFilename} {$uuidsFilename} $numProcesses");
+            unlink($scriptFilename);
+
+            // Get the contents of the file
+            $uuids = file($uuidsFilename);
+            unlink($uuidsFilename);
+
+            // Ensure all the UUIDs are unique
+            $this->assertEquals($numProcesses, count(array_unique($uuids)));
+        }
+    }
 }