From 34b9d4dcaf09e6f60867b8ee9126309a5ce9bf54 Mon Sep 17 00:00:00 2001 From: Jeremy Mikola Date: Thu, 29 Nov 2018 16:44:07 -0500 Subject: [PATCH] PHPC-1274: Reset mongoc_client_t for child processes Check pid before operations that could interfere with resources owned by the parent process. This includes freeing Cursor and Session objects, executing operations on the Manager or Server, or iterating a Cursor. --- php_phongo.h | 7 +++ php_phongo_structs.h | 4 ++ src/MongoDB/Cursor.c | 6 +++ src/MongoDB/Manager.c | 9 ++++ src/MongoDB/Server.c | 8 ++++ src/MongoDB/Session.c | 4 ++ tests/cursor/bug1274-001.phpt | 76 +++++++++++++++++++++++++++++++ tests/cursor/bug1274-002.phpt | 81 ++++++++++++++++++++++++++++++++++ tests/session/bug1274-001.phpt | 68 ++++++++++++++++++++++++++++ 9 files changed, 263 insertions(+) create mode 100644 tests/cursor/bug1274-001.phpt create mode 100644 tests/cursor/bug1274-002.phpt create mode 100644 tests/session/bug1274-001.phpt diff --git a/php_phongo.h b/php_phongo.h index 72eea9912..68e840a87 100644 --- a/php_phongo.h +++ b/php_phongo.h @@ -212,6 +212,13 @@ zend_bool phongo_writeconcernerror_init(zval* return_value, bson_t* bson TSRMLS_ #define PHONGO_ZVAL_CLASS_OR_TYPE_NAME(zv) (Z_TYPE(zv) == IS_OBJECT ? ZSTR_VAL(Z_OBJCE(zv)->name) : zend_get_type_by_const(Z_TYPE(zv))) #define PHONGO_ZVAL_CLASS_OR_TYPE_NAME_P(zvp) PHONGO_ZVAL_CLASS_OR_TYPE_NAME(*(zvp)) +#define PHONGO_CLIENT_RESET_IF_CHIlD_PID(client, created_by_pid) \ + do { \ + if ((created_by_pid) != getpid()) { \ + mongoc_client_reset(client); \ + } \ + } while (0); + #endif /* PHONGO_H */ /* diff --git a/php_phongo_structs.h b/php_phongo_structs.h index 856fb7684..7141a1ef9 100644 --- a/php_phongo_structs.h +++ b/php_phongo_structs.h @@ -55,6 +55,7 @@ typedef struct { PHONGO_ZEND_OBJECT_PRE mongoc_cursor_t* cursor; mongoc_client_t* client; + int created_by_pid; uint32_t server_id; bool advanced; php_phongo_bson_state visitor_data; @@ -78,6 +79,7 @@ typedef struct { typedef struct { PHONGO_ZEND_OBJECT_PRE mongoc_client_t* client; + int created_by_pid; PHONGO_ZEND_OBJECT_POST } php_phongo_manager_t; @@ -108,12 +110,14 @@ typedef struct { PHONGO_ZEND_OBJECT_PRE mongoc_client_t* client; uint32_t server_id; + int created_by_pid; PHONGO_ZEND_OBJECT_POST } php_phongo_server_t; typedef struct { PHONGO_ZEND_OBJECT_PRE mongoc_client_session_t* client_session; + int created_by_pid; PHONGO_ZEND_OBJECT_POST } php_phongo_session_t; diff --git a/src/MongoDB/Cursor.c b/src/MongoDB/Cursor.c index 9d697af21..a3cfdcdf8 100644 --- a/src/MongoDB/Cursor.c +++ b/src/MongoDB/Cursor.c @@ -122,6 +122,8 @@ static void php_phongo_cursor_iterator_move_forward(zend_object_iterator* iter T cursor->advanced = true; } + PHONGO_CLIENT_RESET_IF_CHIlD_PID(cursor->client, cursor->created_by_pid); + if (mongoc_cursor_next(cursor->cursor, &doc)) { php_phongo_bson_to_zval_ex(bson_get_data(doc), doc->len, &cursor->visitor_data); } else { @@ -382,6 +384,8 @@ static void php_phongo_cursor_free_object(phongo_free_object_arg* object TSRMLS_ zend_object_std_dtor(&intern->std TSRMLS_CC); + PHONGO_CLIENT_RESET_IF_CHIlD_PID(intern->client, intern->created_by_pid); + if (intern->cursor) { mongoc_cursor_destroy(intern->cursor); } @@ -428,6 +432,8 @@ static phongo_create_object_retval php_phongo_cursor_create_object(zend_class_en zend_object_std_init(&intern->std, class_type TSRMLS_CC); object_properties_init(&intern->std, class_type); + intern->created_by_pid = (int) getpid(); + #if PHP_VERSION_ID >= 70000 intern->std.handlers = &php_phongo_handler_cursor; diff --git a/src/MongoDB/Manager.c b/src/MongoDB/Manager.c index 2a406cc86..c7531a3b6 100644 --- a/src/MongoDB/Manager.c +++ b/src/MongoDB/Manager.c @@ -357,6 +357,7 @@ static PHP_METHOD(Manager, executeCommand) goto cleanup; } + PHONGO_CLIENT_RESET_IF_CHIlD_PID(intern->client, intern->created_by_pid); phongo_execute_command(intern->client, PHONGO_COMMAND_RAW, db, command, options, server_id, return_value, return_value_used TSRMLS_CC); cleanup: @@ -394,6 +395,7 @@ static PHP_METHOD(Manager, executeReadCommand) return; } + PHONGO_CLIENT_RESET_IF_CHIlD_PID(intern->client, intern->created_by_pid); phongo_execute_command(intern->client, PHONGO_COMMAND_READ, db, command, options, server_id, return_value, return_value_used TSRMLS_CC); } /* }}} */ @@ -420,6 +422,7 @@ static PHP_METHOD(Manager, executeWriteCommand) return; } + PHONGO_CLIENT_RESET_IF_CHIlD_PID(intern->client, intern->created_by_pid); phongo_execute_command(intern->client, PHONGO_COMMAND_WRITE, db, command, options, server_id, return_value, return_value_used TSRMLS_CC); } /* }}} */ @@ -446,6 +449,7 @@ static PHP_METHOD(Manager, executeReadWriteCommand) return; } + PHONGO_CLIENT_RESET_IF_CHIlD_PID(intern->client, intern->created_by_pid); phongo_execute_command(intern->client, PHONGO_COMMAND_READ_WRITE, db, command, options, server_id, return_value, return_value_used TSRMLS_CC); } /* }}} */ @@ -481,6 +485,7 @@ static PHP_METHOD(Manager, executeQuery) goto cleanup; } + PHONGO_CLIENT_RESET_IF_CHIlD_PID(intern->client, intern->created_by_pid); phongo_execute_query(intern->client, namespace, query, options, server_id, return_value, return_value_used TSRMLS_CC); cleanup: @@ -517,6 +522,7 @@ static PHP_METHOD(Manager, executeBulkWrite) goto cleanup; } + PHONGO_CLIENT_RESET_IF_CHIlD_PID(intern->client, intern->created_by_pid); phongo_execute_bulk_write(intern->client, namespace, bulk, options, server_id, return_value, return_value_used TSRMLS_CC); cleanup: @@ -690,6 +696,7 @@ static PHP_METHOD(Manager, startSession) } } + PHONGO_CLIENT_RESET_IF_CHIlD_PID(intern->client, intern->created_by_pid); cs = mongoc_client_start_session(intern->client, cs_opts, &error); if (cs) { @@ -795,6 +802,8 @@ static phongo_create_object_retval php_phongo_manager_create_object(zend_class_e zend_object_std_init(&intern->std, class_type TSRMLS_CC); object_properties_init(&intern->std, class_type); + intern->created_by_pid = (int) getpid(); + #if PHP_VERSION_ID >= 70000 intern->std.handlers = &php_phongo_handler_manager; diff --git a/src/MongoDB/Server.c b/src/MongoDB/Server.c index e735ad5f5..ffacdce79 100644 --- a/src/MongoDB/Server.c +++ b/src/MongoDB/Server.c @@ -47,6 +47,7 @@ static PHP_METHOD(Server, executeCommand) options = php_phongo_prep_legacy_option(options, "readPreference", &free_options TSRMLS_CC); + PHONGO_CLIENT_RESET_IF_CHIlD_PID(intern->client, intern->created_by_pid); phongo_execute_command(intern->client, PHONGO_COMMAND_RAW, db, command, options, intern->server_id, return_value, return_value_used TSRMLS_CC); if (free_options) { @@ -71,6 +72,7 @@ static PHP_METHOD(Server, executeReadCommand) return; } + PHONGO_CLIENT_RESET_IF_CHIlD_PID(intern->client, intern->created_by_pid); phongo_execute_command(intern->client, PHONGO_COMMAND_READ, db, command, options, intern->server_id, return_value, return_value_used TSRMLS_CC); } /* }}} */ @@ -91,6 +93,7 @@ static PHP_METHOD(Server, executeWriteCommand) return; } + PHONGO_CLIENT_RESET_IF_CHIlD_PID(intern->client, intern->created_by_pid); phongo_execute_command(intern->client, PHONGO_COMMAND_WRITE, db, command, options, intern->server_id, return_value, return_value_used TSRMLS_CC); } /* }}} */ @@ -111,6 +114,7 @@ static PHP_METHOD(Server, executeReadWriteCommand) return; } + PHONGO_CLIENT_RESET_IF_CHIlD_PID(intern->client, intern->created_by_pid); phongo_execute_command(intern->client, PHONGO_COMMAND_READ_WRITE, db, command, options, intern->server_id, return_value, return_value_used TSRMLS_CC); } /* }}} */ @@ -134,6 +138,7 @@ static PHP_METHOD(Server, executeQuery) options = php_phongo_prep_legacy_option(options, "readPreference", &free_options TSRMLS_CC); + PHONGO_CLIENT_RESET_IF_CHIlD_PID(intern->client, intern->created_by_pid); phongo_execute_query(intern->client, namespace, query, options, intern->server_id, return_value, return_value_used TSRMLS_CC); if (free_options) { @@ -165,6 +170,7 @@ static PHP_METHOD(Server, executeBulkWrite) options = php_phongo_prep_legacy_option(options, "writeConcern", &free_options TSRMLS_CC); + PHONGO_CLIENT_RESET_IF_CHIlD_PID(intern->client, intern->created_by_pid); phongo_execute_bulk_write(intern->client, namespace, bulk, options, intern->server_id, return_value, return_value_used TSRMLS_CC); if (free_options) { @@ -570,6 +576,8 @@ static phongo_create_object_retval php_phongo_server_create_object(zend_class_en zend_object_std_init(&intern->std, class_type TSRMLS_CC); object_properties_init(&intern->std, class_type); + intern->created_by_pid = (int) getpid(); + #if PHP_VERSION_ID >= 70000 intern->std.handlers = &php_phongo_handler_server; diff --git a/src/MongoDB/Session.c b/src/MongoDB/Session.c index 0c96df6cc..80e09b701 100644 --- a/src/MongoDB/Session.c +++ b/src/MongoDB/Session.c @@ -450,6 +450,8 @@ static void php_phongo_session_free_object(phongo_free_object_arg* object TSRMLS zend_object_std_dtor(&intern->std TSRMLS_CC); + PHONGO_CLIENT_RESET_IF_CHIlD_PID(mongoc_client_session_get_client(intern->client_session), intern->created_by_pid); + if (intern->client_session) { mongoc_client_session_destroy(intern->client_session); } @@ -468,6 +470,8 @@ static phongo_create_object_retval php_phongo_session_create_object(zend_class_e zend_object_std_init(&intern->std, class_type TSRMLS_CC); object_properties_init(&intern->std, class_type); + intern->created_by_pid = (int) getpid(); + #if PHP_VERSION_ID >= 70000 intern->std.handlers = &php_phongo_handler_session; diff --git a/tests/cursor/bug1274-001.phpt b/tests/cursor/bug1274-001.phpt new file mode 100644 index 000000000..aced684de --- /dev/null +++ b/tests/cursor/bug1274-001.phpt @@ -0,0 +1,76 @@ +--TEST-- +PHPC-1274: Cursor destruct should not kill cursor from parent process +--SKIPIF-- + + + + +--FILE-- +getCommand(); + + if ($event->getCommandName() === 'find') { + printf("find command specifies batchSize: %d\n", $command->batchSize); + } + + if ($event->getCommandName() === 'getMore') { + printf("getMore command specifies batchSize: %d\n", $command->batchSize); + } + } + + public function commandSucceeded(MongoDB\Driver\Monitoring\CommandSucceededEvent $event) + { + } + + public function commandFailed(MongoDB\Driver\Monitoring\CommandFailedEvent $event) + { + } +} + +MongoDB\Driver\Monitoring\addSubscriber(new CommandLogger); + +$manager = new MongoDB\Driver\Manager(URI); + +$bulk = new MongoDB\Driver\BulkWrite(); +$bulk->insert(['x' => 1]); +$bulk->insert(['x' => 2]); +$bulk->insert(['x' => 3]); +$manager->executeBulkWrite(NS, $bulk); + +$query = new MongoDB\Driver\Query([], ['batchSize' => 2]); +$cursor = $manager->executeQuery(NS, $query); + +$parentPid = getmypid(); +$childPid = pcntl_fork(); + +if ($childPid === 0) { + echo "Child exits\n"; + exit; +} + +if ($childPid > 0) { + $waitPid = pcntl_waitpid($childPid, $status); + + if ($waitPid === $childPid) { + echo "Parent waited for child to exit\n"; + } + + printf("Parent fully iterated cursor for %d documents\n", iterator_count($cursor)); +} + +?> +===DONE=== + +--EXPECT-- +find command specifies batchSize: 2 +Child exits +Parent waited for child to exit +getMore command specifies batchSize: 2 +Parent fully iterated cursor for 3 documents +===DONE=== diff --git a/tests/cursor/bug1274-002.phpt b/tests/cursor/bug1274-002.phpt new file mode 100644 index 000000000..eacf2b124 --- /dev/null +++ b/tests/cursor/bug1274-002.phpt @@ -0,0 +1,81 @@ +--TEST-- +PHPC-1274: Child process cannot iterate cursor from parent process +--SKIPIF-- + + + + +--FILE-- +getCommand(); + + if ($event->getCommandName() === 'find') { + printf("find command specifies batchSize: %d\n", $command->batchSize); + } + + if ($event->getCommandName() === 'getMore') { + printf("getMore command specifies batchSize: %d\n", $command->batchSize); + } + } + + public function commandSucceeded(MongoDB\Driver\Monitoring\CommandSucceededEvent $event) + { + } + + public function commandFailed(MongoDB\Driver\Monitoring\CommandFailedEvent $event) + { + } +} + +MongoDB\Driver\Monitoring\addSubscriber(new CommandLogger); + +$manager = new MongoDB\Driver\Manager(URI); + +$bulk = new MongoDB\Driver\BulkWrite(); +$bulk->insert(['x' => 1]); +$bulk->insert(['x' => 2]); +$bulk->insert(['x' => 3]); +$manager->executeBulkWrite(NS, $bulk); + +$query = new MongoDB\Driver\Query([], ['batchSize' => 2]); +$cursor = $manager->executeQuery(NS, $query); + +$parentPid = getmypid(); +$childPid = pcntl_fork(); + +if ($childPid === 0) { + echo throws(function() use ($cursor) { + printf("Child fully iterated cursor for %d documents\n", iterator_count($cursor)); + }, 'MongoDB\Driver\Exception\RuntimeException'), "\n"; + echo "Child exits\n"; + exit; +} + +if ($childPid > 0) { + $waitPid = pcntl_waitpid($childPid, $status); + + if ($waitPid === $childPid) { + echo "Parent waited for child to exit\n"; + } + + printf("Parent fully iterated cursor for %d documents\n", iterator_count($cursor)); +} + +?> +===DONE=== + +--EXPECT-- +find command specifies batchSize: 2 +OK: Got MongoDB\Driver\Exception\RuntimeException +Cannot advance cursor after client reset +Child exits +Parent waited for child to exit +getMore command specifies batchSize: 2 +Parent fully iterated cursor for 3 documents +===DONE=== diff --git a/tests/session/bug1274-001.phpt b/tests/session/bug1274-001.phpt new file mode 100644 index 000000000..250246ab3 --- /dev/null +++ b/tests/session/bug1274-001.phpt @@ -0,0 +1,68 @@ +--TEST-- +PHPC-1274: Session destruct should not kill session from parent process +--SKIPIF-- + + + + + + +--FILE-- +executeCommand( + DATABASE_NAME, + new MongoDB\Driver\Command(['create' => COLLECTION_NAME]), + ['writeConcern' => new MongoDB\Driver\WriteConcern('majority')] +); + +$session = $manager->startSession(); +$session->startTransaction(['writeConcern' => new MongoDB\Driver\WriteConcern('majority')]); + +$bulk = new MongoDB\Driver\BulkWrite(); +$bulk->insert(['x' => 1]); +$bulk->insert(['x' => 2]); +$result = $manager->executeBulkWrite(NS, $bulk, ['session' => $session]); +printf("Parent inserted %d documents\n", $result->getInsertedCount()); + +$parentPid = getmypid(); +$childPid = pcntl_fork(); + +if ($childPid === 0) { + echo "Child exits\n"; + exit; +} + +if ($childPid > 0) { + $waitPid = pcntl_waitpid($childPid, $status); + + if ($waitPid === $childPid) { + echo "Parent waited for child to exit\n"; + } + + $bulk = new MongoDB\Driver\BulkWrite(); + $bulk->insert(['x' => 3]); + $bulk->insert(['x' => 4]); + $result = $manager->executeBulkWrite(NS, $bulk, ['session' => $session]); + printf("Parent inserted %d documents\n", $result->getInsertedCount()); + + $session->commitTransaction(); + + $cursor = $manager->executeQuery(NS, new MongoDB\Driver\Query([])); + printf("Parent fully iterated cursor for %d documents\n", iterator_count($cursor)); +} + +?> +===DONE=== + +--EXPECT-- +Parent inserted 2 documents +Child exits +Parent waited for child to exit +Parent inserted 2 documents +Parent fully iterated cursor for 4 documents +===DONE===