Skip to content

Commit 427854e

Browse files
jmikolajmikola
committed
PHPC-1152: Create implicit session for commands
This ensures that phongo_execute_command creates an implicit session (if supported and not explicit session was provided). In turn, this ensures that any command cursor shares the same session as its originating command. By creating a Session object, we can ensure that the implicit session is destroyed during garbage collection when the last reference is removed.
1 parent aec79b2 commit 427854e

File tree

5 files changed

+321
-2
lines changed

5 files changed

+321
-2
lines changed

‎php_phongo.c‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -827,6 +827,28 @@ static bson_t *create_wrapped_command_envelope(const char *db, bson_t *reply)
827827
return tmp;
828828
}
829829

830+
static zval *phongo_create_implicit_session(mongoc_client_t *client TSRMLS_DC) /* {{{ */
831+
{
832+
mongoc_client_session_t *cs;
833+
zval *zsession;
834+
835+
cs = mongoc_client_start_session(client, NULL, NULL);
836+
837+
if (!cs) {
838+
return NULL;
839+
}
840+
841+
#if PHP_VERSION_ID >= 70000
842+
zsession = ecalloc(sizeof(zval), 1);
843+
#else
844+
ALLOC_INIT_ZVAL(zsession);
845+
#endif
846+
847+
phongo_session_init(zsession, cs TSRMLS_CC);
848+
849+
return zsession;
850+
} /* }}} */
851+
830852
bool phongo_execute_command(mongoc_client_t *client, php_phongo_command_type_t type, const char *db, zval *zcommand, zval *options, uint32_t server_id, zval *return_value, int return_value_used TSRMLS_DC) /* {{{ */
831853
{
832854
const php_phongo_command_t *command;
@@ -839,6 +861,7 @@ bool phongo_execute_command(mongoc_client_t *client, php_phongo_command_type_t t
839861
zval *zsession = NULL;
840862
bool result = false;
841863
bool free_reply = false;
864+
bool free_zsession = false;
842865

843866
command = Z_COMMAND_OBJ_P(zcommand);
844867

@@ -857,6 +880,21 @@ bool phongo_execute_command(mongoc_client_t *client, php_phongo_command_type_t t
857880
goto cleanup;
858881
}
859882

883+
/* If an explicit session was not provided, attempt to create an implicit
884+
* client session (ignoring any errors). */
885+
if (!zsession) {
886+
zsession = phongo_create_implicit_session(client TSRMLS_CC);
887+
888+
if (zsession) {
889+
free_zsession = true;
890+
891+
if (!mongoc_client_session_append(Z_SESSION_OBJ_P(zsession)->client_session, &opts, NULL)) {
892+
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT TSRMLS_CC, "Error appending implicit \"sessionId\" option");
893+
goto cleanup;
894+
}
895+
}
896+
}
897+
860898
if ((type & PHONGO_OPTION_WRITE_CONCERN) && !phongo_parse_write_concern(options, &opts, NULL TSRMLS_CC)) {
861899
/* Exception should already have been thrown */
862900
goto cleanup;
@@ -905,6 +943,7 @@ bool phongo_execute_command(mongoc_client_t *client, php_phongo_command_type_t t
905943
* is ultimately destroyed on both success and failure. */
906944
if (bson_iter_init_find(&iter, &reply, "cursor") && BSON_ITER_HOLDS_DOCUMENT(&iter)) {
907945
bson_t initial_reply = BSON_INITIALIZER;
946+
bson_error_t error = {0};
908947

909948
bson_copy_to(&reply, &initial_reply);
910949

@@ -918,6 +957,13 @@ bool phongo_execute_command(mongoc_client_t *client, php_phongo_command_type_t t
918957
bson_append_int64(&initial_reply, "batchSize", -1, command->batch_size);
919958
}
920959

960+
if (zsession && !mongoc_client_session_append(Z_SESSION_OBJ_P(zsession)->client_session, &initial_reply, &error)) {
961+
phongo_throw_exception_from_bson_error_t(&error TSRMLS_CC);
962+
bson_destroy(&initial_reply);
963+
result = false;
964+
goto cleanup;
965+
}
966+
921967
cmd_cursor = mongoc_cursor_new_from_command_reply(client, &initial_reply, server_id);
922968
} else {
923969
bson_t *wrapped_reply = create_wrapped_command_envelope(db, &reply);
@@ -934,6 +980,15 @@ bool phongo_execute_command(mongoc_client_t *client, php_phongo_command_type_t t
934980
bson_destroy(&reply);
935981
}
936982

983+
if (free_zsession) {
984+
#if PHP_VERSION_ID >= 70000
985+
zval_ptr_dtor(zsession);
986+
efree(zsession);
987+
#else
988+
zval_ptr_dtor(&zsession);
989+
#endif
990+
}
991+
937992
return result;
938993
} /* }}} */
939994
/* }}} */

‎tests/cursor/bug1152-001.phpt‎

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
--TEST--
2+
PHPC-1152: Command cursors should use the same session for getMore and killCursors (implicit)
3+
--SKIPIF--
4+
<?php if (PHP_INT_SIZE !== 8) { die("skip Can't represent 64-bit ints on a 32-bit platform"); } ?>
5+
<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
6+
<?php NEEDS_CRYPTO(); ?>
7+
<?php NEEDS('STANDALONE'); ?>
8+
<?php NEEDS_ATLEAST_MONGODB_VERSION(STANDALONE, "3.6"); ?>
9+
<?php CLEANUP(STANDALONE); ?>
10+
--FILE--
11+
<?php
12+
require_once __DIR__ . "/../utils/basic.inc";
13+
14+
class Test implements MongoDB\Driver\Monitoring\CommandSubscriber
15+
{
16+
private $lsidByCursorId = [];
17+
private $lsidByRequestId = [];
18+
19+
public function executeCommand()
20+
{
21+
$manager = new MongoDB\Driver\Manager(STANDALONE);
22+
23+
$bulk = new MongoDB\Driver\BulkWrite;
24+
$bulk->insert(['_id' => 1]);
25+
$bulk->insert(['_id' => 2]);
26+
$bulk->insert(['_id' => 3]);
27+
$manager->executeBulkWrite(NS, $bulk);
28+
29+
$command = new MongoDB\Driver\Command([
30+
'aggregate' => COLLECTION_NAME,
31+
'pipeline' => [['$match' => new stdClass]],
32+
'cursor' => ['batchSize' => 2],
33+
]);
34+
35+
MongoDB\Driver\Monitoring\addSubscriber($this);
36+
37+
/* By creating two cursors with the same name, PHP's reference counting
38+
* will destroy the first after the second is created. Note that
39+
* mongoc_cursor_destroy also destroys implicit sessions and returns
40+
* them to the LIFO pool. This sequencing allows us to test that getMore
41+
* and killCursors use the session ID corresponding to the original
42+
* aggregate command. */
43+
$cursor = $manager->executeCommand(DATABASE_NAME, $command);
44+
$cursor->toArray();
45+
46+
$cursor = $manager->executeCommand(DATABASE_NAME, $command);
47+
$cursor->toArray();
48+
49+
$cursor = $manager->executeCommand(DATABASE_NAME, $command);
50+
$cursor = $manager->executeCommand(DATABASE_NAME, $command);
51+
unset($cursor);
52+
53+
MongoDB\Driver\Monitoring\removeSubscriber($this);
54+
55+
/* We should expect two unique session IDs over the course of the test,
56+
* since at most two implicit sessions would have been in use at any
57+
* given time. */
58+
printf("Unique session IDs used: %d\n", count(array_unique($this->lsidByRequestId)));
59+
}
60+
61+
public function commandStarted(MongoDB\Driver\Monitoring\CommandStartedEvent $event)
62+
{
63+
$requestId = $event->getRequestId();
64+
$sessionId = bin2hex((string) $event->getCommand()->lsid->id);
65+
66+
printf("%s session ID: %s\n", $event->getCommandName(), $sessionId);
67+
68+
if ($event->getCommandName() === 'aggregate') {
69+
if (isset($this->lsidByRequestId[$requestId])) {
70+
throw new UnexpectedValueException('Previous command observed for request ID: ' . $requestId);
71+
}
72+
73+
$this->lsidByRequestId[$requestId] = $sessionId;
74+
}
75+
76+
if ($event->getCommandName() === 'getMore') {
77+
$cursorId = $event->getCommand()->getMore;
78+
79+
if ( ! isset($this->lsidByCursorId[$cursorId])) {
80+
throw new UnexpectedValueException('No previous command observed for cursor ID: ' . $cursorId);
81+
}
82+
83+
printf("getMore used same session as aggregate: %s\n", $sessionId === $this->lsidByCursorId[$cursorId] ? 'yes' : 'no');
84+
}
85+
86+
if ($event->getCommandName() === 'killCursors') {
87+
$cursorId = $event->getCommand()->cursors[0];
88+
89+
if ( ! isset($this->lsidByCursorId[$cursorId])) {
90+
throw new UnexpectedValueException('No previous command observed for cursor ID: ' . $cursorId);
91+
}
92+
93+
printf("killCursors used same session as aggregate: %s\n", $sessionId === $this->lsidByCursorId[$cursorId] ? 'yes' : 'no');
94+
}
95+
}
96+
97+
public function commandSucceeded(MongoDB\Driver\Monitoring\CommandSucceededEvent $event)
98+
{
99+
/* Associate the aggregate's session ID with its cursor ID so it can be
100+
* looked up by the subsequent getMore or killCursors */
101+
if ($event->getCommandName() === 'aggregate') {
102+
$cursorId = $event->getReply()->cursor->id;
103+
$requestId = $event->getRequestId();
104+
105+
$this->lsidByCursorId[$cursorId] = $this->lsidByRequestId[$requestId];
106+
}
107+
}
108+
109+
public function commandFailed(MongoDB\Driver\Monitoring\CommandFailedEvent $event)
110+
{
111+
}
112+
}
113+
114+
(new Test)->executeCommand();
115+
116+
?>
117+
===DONE===
118+
<?php exit(0); ?>
119+
--EXPECTF--
120+
aggregate session ID: %x
121+
getMore session ID: %x
122+
getMore used same session as aggregate: yes
123+
aggregate session ID: %x
124+
getMore session ID: %x
125+
getMore used same session as aggregate: yes
126+
aggregate session ID: %x
127+
aggregate session ID: %x
128+
killCursors session ID: %x
129+
killCursors used same session as aggregate: yes
130+
killCursors session ID: %x
131+
killCursors used same session as aggregate: yes
132+
Unique session IDs used: 2
133+
===DONE===

‎tests/cursor/bug1152-002.phpt‎

Lines changed: 131 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,131 @@
1+
--TEST--
2+
PHPC-1152: Command cursors should use the same session for getMore and killCursors (explicit)
3+
--SKIPIF--
4+
<?php if (PHP_INT_SIZE !== 8) { die("skip Can't represent 64-bit ints on a 32-bit platform"); } ?>
5+
<?php require __DIR__ . "/../utils/basic-skipif.inc"; ?>
6+
<?php NEEDS_CRYPTO(); ?>
7+
<?php NEEDS('STANDALONE'); ?>
8+
<?php NEEDS_ATLEAST_MONGODB_VERSION(STANDALONE, "3.6"); ?>
9+
<?php CLEANUP(STANDALONE); ?>
10+
--FILE--
11+
<?php
12+
require_once __DIR__ . "/../utils/basic.inc";
13+
14+
class Test implements MongoDB\Driver\Monitoring\CommandSubscriber
15+
{
16+
private $lsidByCursorId = [];
17+
private $lsidByRequestId = [];
18+
19+
public function executeCommand()
20+
{
21+
$manager = new MongoDB\Driver\Manager(STANDALONE);
22+
23+
$bulk = new MongoDB\Driver\BulkWrite;
24+
$bulk->insert(['_id' => 1]);
25+
$bulk->insert(['_id' => 2]);
26+
$bulk->insert(['_id' => 3]);
27+
$manager->executeBulkWrite(NS, $bulk);
28+
29+
$command = new MongoDB\Driver\Command([
30+
'aggregate' => COLLECTION_NAME,
31+
'pipeline' => [['$match' => new stdClass]],
32+
'cursor' => ['batchSize' => 2],
33+
]);
34+
35+
$session = $manager->startSession();
36+
37+
MongoDB\Driver\Monitoring\addSubscriber($this);
38+
39+
/* This uses the same sequencing as the implicit session test; however,
40+
* we should expect all commands (aggregate, getMore, and killCursors)
41+
* to use the same explicit session ID. */
42+
$cursor = $manager->executeCommand(DATABASE_NAME, $command, ['session' => $session]);
43+
$cursor->toArray();
44+
45+
$cursor = $manager->executeCommand(DATABASE_NAME, $command, ['session' => $session]);
46+
$cursor->toArray();
47+
48+
$cursor = $manager->executeCommand(DATABASE_NAME, $command, ['session' => $session]);
49+
$cursor = $manager->executeCommand(DATABASE_NAME, $command, ['session' => $session]);
50+
unset($cursor);
51+
52+
MongoDB\Driver\Monitoring\removeSubscriber($this);
53+
54+
/* We should expect one unique session ID over the course of the test,
55+
* since all commands used the same explicit session. */
56+
printf("Unique session IDs used: %d\n", count(array_unique($this->lsidByRequestId)));
57+
}
58+
59+
public function commandStarted(MongoDB\Driver\Monitoring\CommandStartedEvent $event)
60+
{
61+
$requestId = $event->getRequestId();
62+
$sessionId = bin2hex((string) $event->getCommand()->lsid->id);
63+
64+
printf("%s session ID: %s\n", $event->getCommandName(), $sessionId);
65+
66+
if ($event->getCommandName() === 'aggregate') {
67+
if (isset($this->lsidByRequestId[$requestId])) {
68+
throw new UnexpectedValueException('Previous command observed for request ID: ' . $requestId);
69+
}
70+
71+
$this->lsidByRequestId[$requestId] = $sessionId;
72+
}
73+
74+
if ($event->getCommandName() === 'getMore') {
75+
$cursorId = $event->getCommand()->getMore;
76+
77+
if ( ! isset($this->lsidByCursorId[$cursorId])) {
78+
throw new UnexpectedValueException('No previous command observed for cursor ID: ' . $cursorId);
79+
}
80+
81+
printf("getMore used same session as aggregate: %s\n", $sessionId === $this->lsidByCursorId[$cursorId] ? 'yes' : 'no');
82+
}
83+
84+
if ($event->getCommandName() === 'killCursors') {
85+
$cursorId = $event->getCommand()->cursors[0];
86+
87+
if ( ! isset($this->lsidByCursorId[$cursorId])) {
88+
throw new UnexpectedValueException('No previous command observed for cursor ID: ' . $cursorId);
89+
}
90+
91+
printf("killCursors used same session as aggregate: %s\n", $sessionId === $this->lsidByCursorId[$cursorId] ? 'yes' : 'no');
92+
}
93+
}
94+
95+
public function commandSucceeded(MongoDB\Driver\Monitoring\CommandSucceededEvent $event)
96+
{
97+
/* Associate the aggregate's session ID with its cursor ID so it can be
98+
* looked up by the subsequent getMore or killCursors */
99+
if ($event->getCommandName() === 'aggregate') {
100+
$cursorId = $event->getReply()->cursor->id;
101+
$requestId = $event->getRequestId();
102+
103+
$this->lsidByCursorId[$cursorId] = $this->lsidByRequestId[$requestId];
104+
}
105+
}
106+
107+
public function commandFailed(MongoDB\Driver\Monitoring\CommandFailedEvent $event)
108+
{
109+
}
110+
}
111+
112+
(new Test)->executeCommand();
113+
114+
?>
115+
===DONE===
116+
<?php exit(0); ?>
117+
--EXPECTF--
118+
aggregate session ID: %x
119+
getMore session ID: %x
120+
getMore used same session as aggregate: yes
121+
aggregate session ID: %x
122+
getMore session ID: %x
123+
getMore used same session as aggregate: yes
124+
aggregate session ID: %x
125+
aggregate session ID: %x
126+
killCursors session ID: %x
127+
killCursors used same session as aggregate: yes
128+
killCursors session ID: %x
129+
killCursors used same session as aggregate: yes
130+
Unique session IDs used: 1
131+
===DONE===

‎tests/manager/manager-executeCommand-001.phpt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ object(MongoDB\Driver\Cursor)#%d (%d) {
5555
["readPreference"]=>
5656
NULL
5757
["session"]=>
58-
NULL
58+
%a
5959
["isDead"]=>
6060
bool(false)
6161
["currentIndex"]=>

‎tests/server/server-executeCommand-001.phpt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ object(MongoDB\Driver\Cursor)#%d (%d) {
4444
["readPreference"]=>
4545
NULL
4646
["session"]=>
47-
NULL
47+
%a
4848
["isDead"]=>
4949
bool(false)
5050
["currentIndex"]=>

0 commit comments

Comments
 (0)