Skip to content

Commit 600db56

Browse files
jmikolajmikola
committed
PHPC-912: Do not destroy persisted clients created by other processes
1 parent 353c78e commit 600db56

File tree

3 files changed

+118
-29
lines changed

3 files changed

+118
-29
lines changed

‎php_phongo.c

Lines changed: 53 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1517,6 +1517,39 @@ static mongoc_client_t *php_phongo_make_mongo_client(const mongoc_uri_t *uri, mo
15171517
return client;
15181518
} /* }}} */
15191519

1520+
static void php_phongo_persist_client(const char *hash, size_t hash_len, mongoc_client_t *client TSRMLS_DC)
1521+
{
1522+
php_phongo_pclient_t *pclient = (php_phongo_pclient_t *) pecalloc(1, sizeof(php_phongo_pclient_t), 1);
1523+
1524+
pclient->pid = (int) getpid();
1525+
pclient->client = client;
1526+
1527+
#if PHP_VERSION_ID >= 70000
1528+
zend_hash_str_update_ptr(&MONGODB_G(pclients), hash, hash_len, pclient);
1529+
#else
1530+
zend_hash_update(&MONGODB_G(pclients), hash, hash_len + 1, &pclient, sizeof(php_phongo_pclient_t *), NULL);
1531+
#endif
1532+
}
1533+
1534+
static mongoc_client_t *php_phongo_find_client(const char *hash, size_t hash_len TSRMLS_DC)
1535+
{
1536+
#if PHP_VERSION_ID >= 70000
1537+
php_phongo_pclient_t *pclient;
1538+
1539+
if ((pclient = zend_hash_str_find_ptr(&MONGODB_G(pclients), hash, hash_len)) != NULL) {
1540+
return pclient->client;
1541+
}
1542+
#else
1543+
php_phongo_pclient_t **pclient;
1544+
1545+
if (zend_hash_find(&MONGODB_G(pclients), hash, hash_len + 1, (void**) &pclient) == SUCCESS) {
1546+
return (*pclient)->client;
1547+
}
1548+
#endif
1549+
1550+
return NULL;
1551+
}
1552+
15201553
void phongo_manager_init(php_phongo_manager_t *manager, const char *uri_string, zval *options, zval *driverOptions TSRMLS_DC) /* {{{ */
15211554
{
15221555
char *hash = NULL;
@@ -1526,30 +1559,15 @@ void phongo_manager_init(php_phongo_manager_t *manager, const char *uri_string,
15261559
mongoc_ssl_opt_t *ssl_opt = NULL;
15271560
bson_iter_t iter;
15281561

1529-
#if PHP_VERSION_ID >= 70000
1530-
mongoc_client_t *client_ptr;
1531-
#else
1532-
mongoc_client_t **client_ptr;
1533-
#endif
1534-
15351562
if (!(hash = php_phongo_manager_make_client_hash(uri_string, options, driverOptions, &hash_len TSRMLS_CC))) {
15361563
/* Exception should already have been thrown and there is nothing to free */
15371564
return;
15381565
}
15391566

1540-
#if PHP_VERSION_ID >= 70000
1541-
if ((client_ptr = zend_hash_str_find_ptr(&MONGODB_G(clients), hash, hash_len)) != NULL) {
1542-
MONGOC_DEBUG("Found client for hash: %s\n", hash);
1543-
manager->client = client_ptr;
1544-
goto cleanup;
1545-
}
1546-
#else
1547-
if (zend_hash_find(&MONGODB_G(clients), hash, hash_len + 1, (void**) &client_ptr) == SUCCESS) {
1567+
if ((manager->client = php_phongo_find_client(hash, hash_len TSRMLS_CC))) {
15481568
MONGOC_DEBUG("Found client for hash: %s\n", hash);
1549-
manager->client = *client_ptr;
15501569
goto cleanup;
15511570
}
1552-
#endif
15531571

15541572
if (options) {
15551573
phongo_zval_to_bson(options, PHONGO_BSON_NONE, &bson_options, NULL TSRMLS_CC);
@@ -1596,11 +1614,7 @@ void phongo_manager_init(php_phongo_manager_t *manager, const char *uri_string,
15961614
}
15971615

15981616
MONGOC_DEBUG("Created client hash: %s\n", hash);
1599-
#if PHP_VERSION_ID >= 70000
1600-
zend_hash_str_update_ptr(&MONGODB_G(clients), hash, hash_len, manager->client);
1601-
#else
1602-
zend_hash_update(&MONGODB_G(clients), hash, hash_len + 1, &manager->client, sizeof(mongoc_client_t *), NULL);
1603-
#endif
1617+
php_phongo_persist_client(hash, hash_len, manager->client TSRMLS_CC);
16041618

16051619
cleanup:
16061620
if (hash) {
@@ -1835,15 +1849,26 @@ PHP_INI_BEGIN()
18351849
PHP_INI_END()
18361850
/* }}} */
18371851

1852+
static inline void php_phongo_pclient_destroy(php_phongo_pclient_t *pclient)
1853+
{
1854+
/* Do not destroy mongoc_client_t objects created by other processes. This
1855+
* ensures that we do not shutdown sockets that may still be in use by our
1856+
* parent process (see: CDRIVER-2049). While this is a leak, we are already
1857+
* in MSHUTDOWN at this point. */
1858+
if (pclient->pid == getpid()) {
1859+
mongoc_client_destroy(pclient->client);
1860+
}
1861+
}
1862+
18381863
#if PHP_VERSION_ID >= 70000
1839-
static void php_phongo_client_dtor(zval *zv)
1864+
static void php_phongo_pclient_dtor(zval *zv)
18401865
{
1841-
mongoc_client_destroy((mongoc_client_t *) Z_PTR_P(zv));
1866+
php_phongo_pclient_destroy((php_phongo_pclient_t *) Z_PTR_P(zv));
18421867
}
18431868
#else
1844-
static void php_phongo_client_dtor(void *client)
1869+
static void php_phongo_pclient_dtor(void *pp)
18451870
{
1846-
mongoc_client_destroy(*((mongoc_client_t **) client));
1871+
php_phongo_pclient_destroy(*((php_phongo_pclient_t **) pp));
18471872
}
18481873
#endif
18491874

@@ -1864,7 +1889,7 @@ PHP_GINIT_FUNCTION(mongodb)
18641889
memset(mongodb_globals, 0, sizeof(zend_mongodb_globals));
18651890
mongodb_globals->bsonMemVTable = bsonMemVTable;
18661891
/* Initialize HashTable for persistent clients */
1867-
zend_hash_init_ex(&mongodb_globals->clients, 0, NULL, php_phongo_client_dtor, 1, 0);
1892+
zend_hash_init_ex(&mongodb_globals->pclients, 0, NULL, php_phongo_pclient_dtor, 1, 0);
18681893
}
18691894
/* }}} */
18701895

@@ -1994,8 +2019,8 @@ PHP_MSHUTDOWN_FUNCTION(mongodb)
19942019
(void)type; /* We don't care if we are loaded via dl() or extension= */
19952020

19962021
/* Destroy HashTable for persistent clients. The HashTable destructor will
1997-
* destroy any mongoc_client_t objects contained within. */
1998-
zend_hash_destroy(&MONGODB_G(clients));
2022+
* destroy any mongoc_client_t objects that were created by this process. */
2023+
zend_hash_destroy(&MONGODB_G(pclients));
19992024

20002025
bson_mem_restore_vtable();
20012026
/* Cleanup after libmongoc */

‎php_phongo.h

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,19 @@ extern zend_module_entry mongodb_module_entry;
3838
# define PHONGO_API
3939
#endif
4040

41+
/* Structure for persisted libmongoc clients. The PID is included to ensure that
42+
* processes do not destroy clients created by other processes (relevant for
43+
* forking). We avoid using pid_t for Windows compatibility. */
44+
typedef struct {
45+
mongoc_client_t *client;
46+
int pid;
47+
} php_phongo_pclient_t;
48+
4149
ZEND_BEGIN_MODULE_GLOBALS(mongodb)
4250
char *debug;
4351
FILE *debug_fd;
4452
bson_mem_vtable_t bsonMemVTable;
45-
HashTable clients;
53+
HashTable pclients;
4654
ZEND_END_MODULE_GLOBALS(mongodb)
4755

4856
#if PHP_VERSION_ID >= 70000

‎tests/manager/bug0912-001.phpt

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
--TEST--
2+
PHPC-912: Child process should not destroy mongoc_client_t objects from parent
3+
--SKIPIF--
4+
<?php if (!function_exists('pcntl_fork')) { die('skip pcntl_fork() not available'); } ?>
5+
<?php require __DIR__ . "/../utils/basic-skipif.inc"; CLEANUP(STANDALONE); SLOW(); ?>
6+
--FILE--
7+
<?php
8+
require_once __DIR__ . "/../utils/basic.inc";
9+
10+
function logMyURI(MongoDB\Driver\Manager $manager)
11+
{
12+
$command = new MongoDB\Driver\Command(['whatsmyuri' => 1]);
13+
$cursor = $manager->executeCommand(DATABASE_NAME, $command);
14+
$uri = $cursor->toArray()[0]->you;
15+
16+
$bulk = new MongoDB\Driver\BulkWrite();
17+
$bulk->insert(['pid' => getmypid(), 'uri' => $uri]);
18+
$manager->executeBulkWrite(NS, $bulk);
19+
}
20+
21+
$manager = new MongoDB\Driver\Manager(STANDALONE);
22+
logMyURI($manager);
23+
24+
$parentPid = getmypid();
25+
$childPid = pcntl_fork();
26+
27+
if ($childPid === 0) {
28+
$manager = new MongoDB\Driver\Manager(STANDALONE);
29+
logMyURI($manager);
30+
exit;
31+
}
32+
33+
if ($childPid) {
34+
$waitPid = pcntl_waitpid($childPid, $status);
35+
36+
if ($waitPid > 0) {
37+
printf("Parent(%d) waited for child(%d) to exit\n", $parentPid, $waitPid);
38+
}
39+
40+
$cursor = $manager->executeQuery(NS, new MongoDB\Driver\Query([]));
41+
$results = $cursor->toArray();
42+
43+
printf("%d connections were logged\n", count($results));
44+
printf("PIDs differ: %s\n", $results[0]->pid !== $results[1]->pid ? 'yes' : 'no');
45+
printf("URIs differ: %s\n", $results[0]->uri !== $results[1]->uri ? 'yes' : 'no');
46+
}
47+
48+
?>
49+
===DONE===
50+
<?php exit(0); ?>
51+
--EXPECTF--
52+
Parent(%d) waited for child(%d) to exit
53+
2 connections were logged
54+
PIDs differ: yes
55+
URIs differ: yes
56+
===DONE===

0 commit comments

Comments
 (0)