Skip to content

Commit 21307de

Browse files
jmikolajmikola
committed
PHPC-1151: Maintain Session reference on Cursor
This ensures that the Session and underlying mongoc_client_session_t will not be freed until after the Cursor's mongoc_cursor_t object is destroyed.
1 parent 06f6080 commit 21307de

File tree

7 files changed

+174
-9
lines changed

7 files changed

+174
-9
lines changed

‎php_phongo.c‎

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -238,7 +238,7 @@ static void php_phongo_log(mongoc_log_level_t log_level, const char *log_domain,
238238
/* }}} */
239239

240240
/* {{{ Init objects */
241-
static void phongo_cursor_init(zval *return_value, mongoc_client_t *client, mongoc_cursor_t *cursor, zval *readPreference TSRMLS_DC) /* {{{ */
241+
static void phongo_cursor_init(zval *return_value, mongoc_client_t *client, mongoc_cursor_t *cursor, zval *readPreference, zval *session TSRMLS_DC) /* {{{ */
242242
{
243243
php_phongo_cursor_t *intern;
244244

@@ -256,15 +256,24 @@ static void phongo_cursor_init(zval *return_value, mongoc_client_t *client, mong
256256
#else
257257
Z_ADDREF_P(readPreference);
258258
intern->read_preference = readPreference;
259+
#endif
260+
}
261+
262+
if (session) {
263+
#if PHP_VERSION_ID >= 70000
264+
ZVAL_ZVAL(&intern->session, session, 1, 0);
265+
#else
266+
Z_ADDREF_P(session);
267+
intern->session = session;
259268
#endif
260269
}
261270
} /* }}} */
262271

263-
static void phongo_cursor_init_for_command(zval *return_value, mongoc_client_t *client, mongoc_cursor_t *cursor, const char *db, zval *command, zval *readPreference TSRMLS_DC) /* {{{ */
272+
static void phongo_cursor_init_for_command(zval *return_value, mongoc_client_t *client, mongoc_cursor_t *cursor, const char *db, zval *command, zval *readPreference, zval *session TSRMLS_DC) /* {{{ */
264273
{
265274
php_phongo_cursor_t *intern;
266275

267-
phongo_cursor_init(return_value, client, cursor, readPreference TSRMLS_CC);
276+
phongo_cursor_init(return_value, client, cursor, readPreference, session TSRMLS_CC);
268277
intern = Z_CURSOR_OBJ_P(return_value);
269278

270279
intern->database = estrdup(db);
@@ -277,11 +286,11 @@ static void phongo_cursor_init_for_command(zval *return_value, mongoc_client_t *
277286
#endif
278287
} /* }}} */
279288

280-
static void phongo_cursor_init_for_query(zval *return_value, mongoc_client_t *client, mongoc_cursor_t *cursor, const char *namespace, zval *query, zval *readPreference TSRMLS_DC) /* {{{ */
289+
static void phongo_cursor_init_for_query(zval *return_value, mongoc_client_t *client, mongoc_cursor_t *cursor, const char *namespace, zval *query, zval *readPreference, zval *session TSRMLS_DC) /* {{{ */
281290
{
282291
php_phongo_cursor_t *intern;
283292

284-
phongo_cursor_init(return_value, client, cursor, readPreference TSRMLS_CC);
293+
phongo_cursor_init(return_value, client, cursor, readPreference, session TSRMLS_CC);
285294
intern = Z_CURSOR_OBJ_P(return_value);
286295

287296
/* namespace has already been validated by phongo_execute_query() */
@@ -748,6 +757,7 @@ int phongo_execute_query(mongoc_client_t *client, const char *namespace, zval *z
748757
char *collname;
749758
mongoc_collection_t *collection;
750759
zval *zreadPreference = NULL;
760+
zval *zsession = NULL;
751761

752762
if (!phongo_split_namespace(namespace, &dbname, &collname)) {
753763
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT TSRMLS_CC, "%s: %s", "Invalid namespace provided", namespace);
@@ -769,7 +779,7 @@ int phongo_execute_query(mongoc_client_t *client, const char *namespace, zval *z
769779
return false;
770780
}
771781

772-
if (!phongo_parse_session(options, client, query->opts, NULL TSRMLS_CC)) {
782+
if (!phongo_parse_session(options, client, query->opts, &zsession TSRMLS_CC)) {
773783
/* Exception should already have been thrown */
774784
mongoc_collection_destroy(collection);
775785
return false;
@@ -799,7 +809,8 @@ int phongo_execute_query(mongoc_client_t *client, const char *namespace, zval *z
799809
return true;
800810
}
801811

802-
phongo_cursor_init_for_query(return_value, client, cursor, namespace, zquery, zreadPreference TSRMLS_CC);
812+
phongo_cursor_init_for_query(return_value, client, cursor, namespace, zquery, zreadPreference, zsession TSRMLS_CC);
813+
803814
return true;
804815
} /* }}} */
805816

@@ -825,6 +836,7 @@ int phongo_execute_command(mongoc_client_t *client, php_phongo_command_type_t ty
825836
bson_t opts = BSON_INITIALIZER;
826837
mongoc_cursor_t *cmd_cursor;
827838
zval *zreadPreference = NULL;
839+
zval *zsession = NULL;
828840
int result;
829841

830842
command = Z_COMMAND_OBJ_P(zcommand);
@@ -841,7 +853,7 @@ int phongo_execute_command(mongoc_client_t *client, php_phongo_command_type_t ty
841853
return false;
842854
}
843855

844-
if (!phongo_parse_session(options, client, &opts, NULL TSRMLS_CC)) {
856+
if (!phongo_parse_session(options, client, &opts, &zsession TSRMLS_CC)) {
845857
/* Exception should already have been thrown */
846858
bson_destroy(&opts);
847859
return false;
@@ -922,7 +934,7 @@ int phongo_execute_command(mongoc_client_t *client, php_phongo_command_type_t ty
922934
bson_destroy(&reply);
923935
}
924936

925-
phongo_cursor_init_for_command(return_value, client, cmd_cursor, db, zcommand, zreadPreference TSRMLS_CC);
937+
phongo_cursor_init_for_command(return_value, client, cmd_cursor, db, zcommand, zreadPreference, zsession TSRMLS_CC);
926938
return true;
927939
} /* }}} */
928940
/* }}} */

‎php_phongo_structs.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ typedef struct {
6565
PHONGO_STRUCT_ZVAL query;
6666
PHONGO_STRUCT_ZVAL command;
6767
PHONGO_STRUCT_ZVAL read_preference;
68+
PHONGO_STRUCT_ZVAL session;
6869
PHONGO_ZEND_OBJECT_POST
6970
} php_phongo_cursor_t;
7071

‎src/MongoDB/Cursor.c‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -392,6 +392,10 @@ static void php_phongo_cursor_free_object(phongo_free_object_arg *object TSRMLS_
392392
zval_ptr_dtor(&intern->read_preference);
393393
}
394394

395+
if (!Z_ISUNDEF(intern->session)) {
396+
zval_ptr_dtor(&intern->session);
397+
}
398+
395399
php_phongo_cursor_free_current(intern);
396400

397401
#if PHP_VERSION_ID < 70000

‎tests/cursor/bug1151-001.phpt‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
--TEST--
2+
PHPC-1151: Segfault if session unset before first getMore (find)
3+
--SKIPIF--
4+
<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
5+
<?php NEEDS_CRYPTO(); ?>
6+
<?php NEEDS('STANDALONE'); ?>
7+
<?php NEEDS_ATLEAST_MONGODB_VERSION(STANDALONE, "3.6"); ?>
8+
<?php CLEANUP(STANDALONE); ?>
9+
--FILE--
10+
<?php
11+
require_once __DIR__ . "/../utils/basic.inc";
12+
13+
$manager = new MongoDB\Driver\Manager(STANDALONE);
14+
15+
$bulk = new MongoDB\Driver\BulkWrite;
16+
$bulk->insert(['_id' => 1]);
17+
$bulk->insert(['_id' => 2]);
18+
$bulk->insert(['_id' => 3]);
19+
$manager->executeBulkWrite(NS, $bulk);
20+
21+
$query = new MongoDB\Driver\Query([], ['batchSize' => 2]);
22+
$session = $manager->startSession();
23+
24+
$cursor = $manager->executeQuery(NS, $query, ['session' => $session]);
25+
26+
foreach ($cursor as $document) {
27+
unset($session);
28+
echo $document->_id, "\n";
29+
}
30+
31+
?>
32+
===DONE===
33+
<?php exit(0); ?>
34+
--EXPECT--
35+
1
36+
2
37+
3
38+
===DONE===

‎tests/cursor/bug1151-002.phpt‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
--TEST--
2+
PHPC-1151: Segfault if session unset before first getMore (aggregate)
3+
--SKIPIF--
4+
<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
5+
<?php NEEDS_CRYPTO(); ?>
6+
<?php NEEDS('STANDALONE'); ?>
7+
<?php NEEDS_ATLEAST_MONGODB_VERSION(STANDALONE, "3.6"); ?>
8+
<?php CLEANUP(STANDALONE); ?>
9+
--FILE--
10+
<?php
11+
require_once __DIR__ . "/../utils/basic.inc";
12+
13+
$manager = new MongoDB\Driver\Manager(STANDALONE);
14+
15+
$bulk = new MongoDB\Driver\BulkWrite;
16+
$bulk->insert(['_id' => 1]);
17+
$bulk->insert(['_id' => 2]);
18+
$bulk->insert(['_id' => 3]);
19+
$manager->executeBulkWrite(NS, $bulk);
20+
21+
$command = new MongoDB\Driver\Command([
22+
'aggregate' => COLLECTION_NAME,
23+
'pipeline' => [],
24+
'cursor' => ['batchSize' => 2],
25+
]);
26+
$session = $manager->startSession();
27+
28+
$cursor = $manager->executeReadCommand(DATABASE_NAME, $command, ['session' => $session]);
29+
30+
foreach ($cursor as $document) {
31+
unset($session);
32+
echo $document->_id, "\n";
33+
}
34+
35+
?>
36+
===DONE===
37+
<?php exit(0); ?>
38+
--EXPECT--
39+
1
40+
2
41+
3
42+
===DONE===

‎tests/cursor/bug1151-003.phpt‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
--TEST--
2+
PHPC-1151: Segfault if session unset before cursor is killed (find)
3+
--SKIPIF--
4+
<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
5+
<?php NEEDS_CRYPTO(); ?>
6+
<?php NEEDS('STANDALONE'); ?>
7+
<?php NEEDS_ATLEAST_MONGODB_VERSION(STANDALONE, "3.6"); ?>
8+
<?php CLEANUP(STANDALONE); ?>
9+
--FILE--
10+
<?php
11+
require_once __DIR__ . "/../utils/basic.inc";
12+
13+
$manager = new MongoDB\Driver\Manager(STANDALONE);
14+
15+
$bulk = new MongoDB\Driver\BulkWrite;
16+
$bulk->insert(['_id' => 1]);
17+
$bulk->insert(['_id' => 2]);
18+
$bulk->insert(['_id' => 3]);
19+
$manager->executeBulkWrite(NS, $bulk);
20+
21+
$query = new MongoDB\Driver\Query([], ['batchSize' => 2]);
22+
$session = $manager->startSession();
23+
24+
$cursor = $manager->executeQuery(NS, $query, ['session' => $session]);
25+
unset($session);
26+
unset($cursor);
27+
28+
?>
29+
===DONE===
30+
<?php exit(0); ?>
31+
--EXPECT--
32+
===DONE===

‎tests/cursor/bug1151-004.phpt‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
--TEST--
2+
PHPC-1151: Segfault if session unset before cursor is killed (aggregate)
3+
--SKIPIF--
4+
<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
5+
<?php NEEDS_CRYPTO(); ?>
6+
<?php NEEDS('STANDALONE'); ?>
7+
<?php NEEDS_ATLEAST_MONGODB_VERSION(STANDALONE, "3.6"); ?>
8+
<?php CLEANUP(STANDALONE); ?>
9+
--FILE--
10+
<?php
11+
require_once __DIR__ . "/../utils/basic.inc";
12+
13+
$manager = new MongoDB\Driver\Manager(STANDALONE);
14+
15+
$bulk = new MongoDB\Driver\BulkWrite;
16+
$bulk->insert(['_id' => 1]);
17+
$bulk->insert(['_id' => 2]);
18+
$bulk->insert(['_id' => 3]);
19+
$manager->executeBulkWrite(NS, $bulk);
20+
21+
$command = new MongoDB\Driver\Command([
22+
'aggregate' => COLLECTION_NAME,
23+
'pipeline' => [],
24+
'cursor' => ['batchSize' => 2],
25+
]);
26+
$session = $manager->startSession();
27+
28+
$cursor = $manager->executeReadCommand(DATABASE_NAME, $command, ['session' => $session]);
29+
unset($session);
30+
unset($cursor);
31+
32+
?>
33+
===DONE===
34+
<?php exit(0); ?>
35+
--EXPECT--
36+
===DONE===

0 commit comments

Comments
 (0)