[PECL-CVS] [pecl-networking-gearman] master: Merge pull request #54 from php/fix-fork-premature-job-completion

Author: Rasmus Lerdorf (rlerdorf)
Committer: GitHub (web-flow)
Pusher: rlerdorf
Date: 2026-04-04T16:00:18+01:00

Commit: https://github.com/php/pecl-networking-gearman/commit/3929944249d545f2fd65abd8e05fd50e57c47bd3
Raw diff: https://github.com/php/pecl-networking-gearman/commit/3929944249d545f2fd65abd8e05fd50e57c47bd3.diff

Merge pull request #54 from php/fix-fork-premature-job-completion

Fixes #40

Changed paths:
  A  tests/gearman_worker_integration_test_003.phpt
  M  php_gearman_client.c
  M  php_gearman_client.h
  M  php_gearman_worker.c
  M  php_gearman_worker.h


Diff:

diff --git a/php_gearman_client.c b/php_gearman_client.c
index 006bb47..5a30ed1 100644
--- a/php_gearman_client.c
+++ b/php_gearman_client.c
@@ -30,6 +30,7 @@ static void gearman_client_ctor(INTERNAL_FUNCTION_PARAMETERS) {
         }
 
         client->flags |= GEARMAN_CLIENT_OBJ_CREATED;
+        client->created_pid = (zend_long)getpid();
         gearman_client_add_options(&(client->client), GEARMAN_CLIENT_FREE_TASKS);
         gearman_client_set_workload_malloc_fn(&(client->client), _php_malloc, NULL);
         gearman_client_set_workload_free_fn(&(client->client), _php_free, NULL);
@@ -82,12 +83,13 @@ PHP_METHOD(GearmanClient, __destruct)
         }
 
         if (intern->flags & GEARMAN_CLIENT_OBJ_CREATED) {
-                context = gearman_client_context(&(intern->client));
-                if (context) {
-                        efree(context);
+                if ((zend_long)getpid() == intern->created_pid) {
+                        context = gearman_client_context(&(intern->client));
+                        if (context) {
+                                efree(context);
+                        }
+                        gearman_client_free(&intern->client);
                 }
-
-                gearman_client_free(&intern->client);
                 intern->flags &= ~GEARMAN_CLIENT_OBJ_CREATED;
         }
 }
diff --git a/php_gearman_client.h b/php_gearman_client.h
index 334282c..c621041 100644
--- a/php_gearman_client.h
+++ b/php_gearman_client.h
@@ -16,6 +16,7 @@
 #include "php_ini.h"
 #include "ext/standard/info.h"
 
+
 #include "zend_exceptions.h"
 #include "zend_interfaces.h"
 
@@ -51,6 +52,7 @@ typedef struct {
 
 	zend_ulong created_tasks;
 	zval task_list;
+	zend_long created_pid;
 
 	zend_object std;
 } gearman_client_obj;
diff --git a/php_gearman_worker.c b/php_gearman_worker.c
index 840b938..1842d80 100644
--- a/php_gearman_worker.c
+++ b/php_gearman_worker.c
@@ -31,6 +31,7 @@ static void gearman_worker_ctor(INTERNAL_FUNCTION_PARAMETERS) {
 	}
 
 	worker->flags |= GEARMAN_WORKER_OBJ_CREATED;
+	worker->created_pid = (zend_long)getpid();
 	gearman_worker_set_workload_malloc_fn(&(worker->worker), _php_malloc, NULL);
 	gearman_worker_set_workload_free_fn(&(worker->worker), _php_free, NULL);
 }
@@ -61,7 +62,14 @@ void gearman_worker_free_obj(zend_object *object) {
         gearman_worker_obj *intern = gearman_worker_fetch_object(object);
 
         if (intern->flags & GEARMAN_WORKER_OBJ_CREATED) {
-                gearman_worker_free(&(intern->worker));
+                /* In forked children, skip gearman_worker_free() to avoid
+                 * sending protocol messages over the parent's connection,
+                 * which would cause gearmand to prematurely mark the
+                 * parent's in-progress job as complete. The inherited fds
+                 * will be closed when the child process exits. See #40. */
+                if ((zend_long)getpid() == intern->created_pid) {
+                        gearman_worker_free(&(intern->worker));
+                }
                 intern->flags &= ~GEARMAN_WORKER_OBJ_CREATED;
         }
 
@@ -80,7 +88,9 @@ PHP_METHOD(GearmanWorker, __destruct) {
 	}
 
 	if (intern->flags & GEARMAN_WORKER_OBJ_CREATED) {
-		gearman_worker_free(&(intern->worker));
+		if ((zend_long)getpid() == intern->created_pid) {
+			gearman_worker_free(&(intern->worker));
+		}
 		intern->flags &= ~GEARMAN_WORKER_OBJ_CREATED;
 	}
 }
diff --git a/php_gearman_worker.h b/php_gearman_worker.h
index 27f94cc..f619001 100644
--- a/php_gearman_worker.h
+++ b/php_gearman_worker.h
@@ -16,6 +16,7 @@
 #include "php_ini.h"
 #include "ext/standard/info.h"
 
+
 #include "zend_exceptions.h"
 #include "zend_interfaces.h"
 
@@ -48,6 +49,7 @@ typedef struct {
         gearman_worker_obj_flags_t flags;
         gearman_worker_st worker;
         zval cb_list;
+        zend_long created_pid;
 
         zend_object std;
 } gearman_worker_obj;
diff --git a/tests/gearman_worker_integration_test_003.phpt b/tests/gearman_worker_integration_test_003.phpt
new file mode 100644
index 0000000..21970aa
--- /dev/null
+++ b/tests/gearman_worker_integration_test_003.phpt
@@ -0,0 +1,65 @@
+--TEST--
+Worker forking children mid-callback does not cause premature job completion (issue #40)
+--SKIPIF--
+<?php
+require_once('skipif.inc');
+require_once('skipifconnect.inc');
+?>
+--FILE--
+<?php
+require_once('connect.inc');
+
+$func = 'issue40_' . getmypid() . '_' . time();
+
+/* Worker in a child so we can be the client in the parent */
+$wpid = pcntl_fork();
+if ($wpid === -1) {
+    die("FAIL: could not fork worker");
+}
+if ($wpid === 0) {
+    $w = new GearmanWorker();
+    if ($w->addServer($host, $port) !== true) exit(2);
+    $w->setTimeout(10000);
+    if ($w->addFunction($func, function($job) {
+        /* Fork two children inside the callback */
+        $children = [];
+        for ($i = 0; $i < 2; $i++) {
+            $pid = pcntl_fork();
+            if ($pid === -1) {
+                continue;
+            } elseif ($pid === 0) {
+                usleep(100000);
+                exit(0);
+            }
+            $children[] = $pid;
+        }
+        foreach ($children as $pid) {
+            pcntl_waitpid($pid, $s, 0);
+        }
+        return "completed";
+    }) !== true) exit(2);
+    $w->work();
+    exit(0);
+}
+
+/* Give the worker time to register */
+usleep(200000);
+
+/* Submit a foreground job and check the result */
+$client = new GearmanClient();
+if ($client->addServer($host, $port) !== true) {
+    die("FAIL: could not add server");
+}
+$client->setTimeout(15000);
+$result = $client->doNormal($func, 'test_payload');
+$rc = $client->returnCode();
+
+pcntl_waitpid($wpid, $ws, 0);
+
+if ($rc === GEARMAN_SUCCESS && $result === "completed") {
+    echo "PASS" . PHP_EOL;
+} else {
+    echo "FAIL: rc=$rc result=" . var_export($result, true) . PHP_EOL;
+}
+--EXPECT--
+PASS