Skip to content

Commit b27017d

Browse files
jmikolajmikola
committed
Merge pull request #803 
2 parents 57d3f6e + 1f222d0 commit b27017d

File tree

11 files changed

+623
-30
lines changed

11 files changed

+623
-30
lines changed

‎php_phongo.c‎

Lines changed: 79 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -749,7 +749,7 @@ bool phongo_cursor_advance_and_check_for_error(mongoc_cursor_t *cursor TSRMLS_DC
749749
return true;
750750
} /* }}} */
751751

752-
int phongo_execute_query(mongoc_client_t *client, const char *namespace, zval *zquery, zval *options, uint32_t server_id, zval *return_value, int return_value_used TSRMLS_DC) /* {{{ */
752+
bool phongo_execute_query(mongoc_client_t *client, const char *namespace, zval *zquery, zval *options, uint32_t server_id, zval *return_value, int return_value_used TSRMLS_DC) /* {{{ */
753753
{
754754
const php_phongo_query_t *query;
755755
mongoc_cursor_t *cursor;
@@ -827,7 +827,29 @@ static bson_t *create_wrapped_command_envelope(const char *db, bson_t *reply)
827827
return tmp;
828828
}
829829

830-
int 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) /* {{{ */
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+
852+
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;
833855
bson_iter_t iter;
@@ -837,38 +859,50 @@ int phongo_execute_command(mongoc_client_t *client, php_phongo_command_type_t ty
837859
mongoc_cursor_t *cmd_cursor;
838860
zval *zreadPreference = NULL;
839861
zval *zsession = NULL;
840-
int result;
862+
bool result = false;
863+
bool free_reply = false;
864+
bool free_zsession = false;
841865

842866
command = Z_COMMAND_OBJ_P(zcommand);
843867

844868
if ((type & PHONGO_OPTION_READ_CONCERN) && !phongo_parse_read_concern(options, &opts TSRMLS_CC)) {
845869
/* Exception should already have been thrown */
846-
bson_destroy(&opts);
847-
return false;
870+
goto cleanup;
848871
}
849872

850873
if ((type & PHONGO_OPTION_READ_PREFERENCE) && !phongo_parse_read_preference(options, &zreadPreference TSRMLS_CC)) {
851874
/* Exception should already have been thrown */
852-
bson_destroy(&opts);
853-
return false;
875+
goto cleanup;
854876
}
855877

856878
if (!phongo_parse_session(options, client, &opts, &zsession TSRMLS_CC)) {
857879
/* Exception should already have been thrown */
858-
bson_destroy(&opts);
859-
return false;
880+
goto cleanup;
881+
}
882+
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+
}
860896
}
861897

862898
if ((type & PHONGO_OPTION_WRITE_CONCERN) && !phongo_parse_write_concern(options, &opts, NULL TSRMLS_CC)) {
863899
/* Exception should already have been thrown */
864-
bson_destroy(&opts);
865-
return false;
900+
goto cleanup;
866901
}
867902

868903
if (!BSON_APPEND_INT32(&opts, "serverId", server_id)) {
869904
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT TSRMLS_CC, "Error appending \"serverId\" option");
870-
bson_destroy(&opts);
871-
return false;
905+
goto cleanup;
872906
}
873907

874908
/* Although "opts" already always includes the serverId option, the read
@@ -891,27 +925,25 @@ int phongo_execute_command(mongoc_client_t *client, php_phongo_command_type_t ty
891925
default:
892926
/* Should never happen, but if it does: exception */
893927
phongo_throw_exception(PHONGO_ERROR_LOGIC TSRMLS_CC, "Type '%d' should never have been passed to phongo_execute_command, please file a bug report", type);
894-
bson_destroy(&opts);
895-
return false;
928+
goto cleanup;
896929
}
930+
931+
free_reply = true;
932+
897933
if (!result) {
898934
phongo_throw_exception_from_bson_error_t(&error TSRMLS_CC);
899-
bson_destroy(&reply);
900-
bson_destroy(&opts);
901-
return false;
935+
goto cleanup;
902936
}
903937

904-
bson_destroy(&opts);
905-
906938
if (!return_value_used) {
907-
bson_destroy(&reply);
908-
return true;
939+
goto cleanup;
909940
}
910941

911942
/* According to mongoc_cursor_new_from_command_reply(), the reply bson_t
912943
* is ultimately destroyed on both success and failure. */
913944
if (bson_iter_init_find(&iter, &reply, "cursor") && BSON_ITER_HOLDS_DOCUMENT(&iter)) {
914945
bson_t initial_reply = BSON_INITIALIZER;
946+
bson_error_t error = {0};
915947

916948
bson_copy_to(&reply, &initial_reply);
917949

@@ -925,17 +957,39 @@ int phongo_execute_command(mongoc_client_t *client, php_phongo_command_type_t ty
925957
bson_append_int64(&initial_reply, "batchSize", -1, command->batch_size);
926958
}
927959

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+
928967
cmd_cursor = mongoc_cursor_new_from_command_reply(client, &initial_reply, server_id);
929-
bson_destroy(&reply);
930968
} else {
931969
bson_t *wrapped_reply = create_wrapped_command_envelope(db, &reply);
932970

933971
cmd_cursor = mongoc_cursor_new_from_command_reply(client, wrapped_reply, server_id);
934-
bson_destroy(&reply);
935972
}
936973

937974
phongo_cursor_init_for_command(return_value, client, cmd_cursor, db, zcommand, zreadPreference, zsession TSRMLS_CC);
938-
return true;
975+
976+
cleanup:
977+
bson_destroy(&opts);
978+
979+
if (free_reply) {
980+
bson_destroy(&reply);
981+
}
982+
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+
992+
return result;
939993
} /* }}} */
940994
/* }}} */
941995

‎php_phongo.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -134,8 +134,8 @@ void phongo_readconcern_init (zval *return_value, const
134134
void phongo_readpreference_init (zval *return_value, const mongoc_read_prefs_t *read_prefs TSRMLS_DC);
135135
void phongo_writeconcern_init (zval *return_value, const mongoc_write_concern_t *write_concern TSRMLS_DC);
136136
bool phongo_execute_bulk_write (mongoc_client_t *client, const char *namespace, php_phongo_bulkwrite_t *bulk_write, zval *zwriteConcern, uint32_t server_id, zval *return_value, int return_value_used TSRMLS_DC);
137-
int phongo_execute_command (mongoc_client_t *client, php_phongo_command_type_t type, const char *db, zval *zcommand, zval *zreadPreference, uint32_t server_id, zval *return_value, int return_value_used TSRMLS_DC);
138-
int phongo_execute_query (mongoc_client_t *client, const char *namespace, zval *zquery, zval *zreadPreference, uint32_t server_id, zval *return_value, int return_value_used TSRMLS_DC);
137+
bool phongo_execute_command (mongoc_client_t *client, php_phongo_command_type_t type, const char *db, zval *zcommand, zval *zreadPreference, uint32_t server_id, zval *return_value, int return_value_used TSRMLS_DC);
138+
bool phongo_execute_query (mongoc_client_t *client, const char *namespace, zval *zquery, zval *zreadPreference, uint32_t server_id, zval *return_value, int return_value_used TSRMLS_DC);
139139

140140
bool phongo_cursor_advance_and_check_for_error(mongoc_cursor_t *cursor TSRMLS_DC);
141141

‎src/MongoDB/Cursor.c‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,22 @@
2828

2929
zend_class_entry *php_phongo_cursor_ce;
3030

31+
/* Check if the cursor is exhausted (i.e. ID is zero) and free any reference to
32+
* the session. Calling this function during iteration will allow an implicit
33+
* session to return to the pool immediately after a getMore indicates that the
34+
* server has no more results to return. */
35+
static void php_phongo_cursor_free_session_if_exhausted(php_phongo_cursor_t *cursor) /* {{{ */
36+
{
37+
if (mongoc_cursor_get_id(cursor->cursor)) {
38+
return;
39+
}
40+
41+
if (!Z_ISUNDEF(cursor->session)) {
42+
zval_ptr_dtor(&cursor->session);
43+
ZVAL_UNDEF(&cursor->session);
44+
}
45+
} /* }}} */
46+
3147
static void php_phongo_cursor_free_current(php_phongo_cursor_t *cursor) /* {{{ */
3248
{
3349
if (!Z_ISUNDEF(cursor->visitor_data.zchild)) {
@@ -117,6 +133,8 @@ static void php_phongo_cursor_iterator_move_forward(zend_object_iterator *iter T
117133
phongo_throw_exception_from_bson_error_t(&error TSRMLS_CC);
118134
}
119135
}
136+
137+
php_phongo_cursor_free_session_if_exhausted(cursor);
120138
} /* }}} */
121139

122140
static void php_phongo_cursor_iterator_rewind(zend_object_iterator *iter TSRMLS_DC) /* {{{ */
@@ -147,6 +165,8 @@ static void php_phongo_cursor_iterator_rewind(zend_object_iterator *iter TSRMLS_
147165
if (doc) {
148166
php_phongo_bson_to_zval_ex(bson_get_data(doc), doc->len, &cursor->visitor_data);
149167
}
168+
169+
php_phongo_cursor_free_session_if_exhausted(cursor);
150170
} /* }}} */
151171

152172
static zend_object_iterator_funcs php_phongo_cursor_iterator_funcs = {
@@ -440,7 +460,7 @@ static HashTable *php_phongo_cursor_get_debug_info(zval *object, int *is_temp TS
440460
*is_temp = 1;
441461
intern = Z_CURSOR_OBJ_P(object);
442462

443-
array_init_size(&retval, 9);
463+
array_init_size(&retval, 10);
444464

445465
if (intern->database) {
446466
ADD_ASSOC_STRING(&retval, "database", intern->database);

‎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===

0 commit comments

Comments
 (0)