Skip to content

Commit 480bc58

Browse files
jmikolajmikola
committed
Merge branch 'v1.4'
2 parents dadf1e6 + b27017d commit 480bc58

11 files changed

+623
-30
lines changed

‎php_phongo.c

Lines changed: 79 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -719,7 +719,7 @@ bool phongo_cursor_advance_and_check_for_error(mongoc_cursor_t* cursor TSRMLS_DC
719719
return true;
720720
} /* }}} */
721721

722-
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) /* {{{ */
722+
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) /* {{{ */
723723
{
724724
const php_phongo_query_t* query;
725725
mongoc_cursor_t* cursor;
@@ -797,7 +797,29 @@ static bson_t* create_wrapped_command_envelope(const char* db, bson_t* reply)
797797
return tmp;
798798
}
799799

800-
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) /* {{{ */
800+
static zval *phongo_create_implicit_session(mongoc_client_t *client TSRMLS_DC) /* {{{ */
801+
{
802+
mongoc_client_session_t *cs;
803+
zval *zsession;
804+
805+
cs = mongoc_client_start_session(client, NULL, NULL);
806+
807+
if (!cs) {
808+
return NULL;
809+
}
810+
811+
#if PHP_VERSION_ID >= 70000
812+
zsession = ecalloc(sizeof(zval), 1);
813+
#else
814+
ALLOC_INIT_ZVAL(zsession);
815+
#endif
816+
817+
phongo_session_init(zsession, cs TSRMLS_CC);
818+
819+
return zsession;
820+
} /* }}} */
821+
822+
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) /* {{{ */
801823
{
802824
const php_phongo_command_t* command;
803825
bson_iter_t iter;
@@ -807,38 +829,50 @@ int phongo_execute_command(mongoc_client_t* client, php_phongo_command_type_t ty
807829
mongoc_cursor_t* cmd_cursor;
808830
zval* zreadPreference = NULL;
809831
zval* zsession = NULL;
810-
int result;
832+
bool result = false;
833+
bool free_reply = false;
834+
bool free_zsession = false;
811835

812836
command = Z_COMMAND_OBJ_P(zcommand);
813837

814838
if ((type & PHONGO_OPTION_READ_CONCERN) && !phongo_parse_read_concern(options, &opts TSRMLS_CC)) {
815839
/* Exception should already have been thrown */
816-
bson_destroy(&opts);
817-
return false;
840+
goto cleanup;
818841
}
819842

820843
if ((type & PHONGO_OPTION_READ_PREFERENCE) && !phongo_parse_read_preference(options, &zreadPreference TSRMLS_CC)) {
821844
/* Exception should already have been thrown */
822-
bson_destroy(&opts);
823-
return false;
845+
goto cleanup;
824846
}
825847

826848
if (!phongo_parse_session(options, client, &opts, &zsession TSRMLS_CC)) {
827849
/* Exception should already have been thrown */
828-
bson_destroy(&opts);
829-
return false;
850+
goto cleanup;
851+
}
852+
853+
/* If an explicit session was not provided, attempt to create an implicit
854+
* client session (ignoring any errors). */
855+
if (!zsession) {
856+
zsession = phongo_create_implicit_session(client TSRMLS_CC);
857+
858+
if (zsession) {
859+
free_zsession = true;
860+
861+
if (!mongoc_client_session_append(Z_SESSION_OBJ_P(zsession)->client_session, &opts, NULL)) {
862+
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT TSRMLS_CC, "Error appending implicit \"sessionId\" option");
863+
goto cleanup;
864+
}
865+
}
830866
}
831867

832868
if ((type & PHONGO_OPTION_WRITE_CONCERN) && !phongo_parse_write_concern(options, &opts, NULL TSRMLS_CC)) {
833869
/* Exception should already have been thrown */
834-
bson_destroy(&opts);
835-
return false;
870+
goto cleanup;
836871
}
837872

838873
if (!BSON_APPEND_INT32(&opts, "serverId", server_id)) {
839874
phongo_throw_exception(PHONGO_ERROR_INVALID_ARGUMENT TSRMLS_CC, "Error appending \"serverId\" option");
840-
bson_destroy(&opts);
841-
return false;
875+
goto cleanup;
842876
}
843877

844878
/* Although "opts" already always includes the serverId option, the read
@@ -861,9 +895,11 @@ int phongo_execute_command(mongoc_client_t* client, php_phongo_command_type_t ty
861895
default:
862896
/* Should never happen, but if it does: exception */
863897
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);
864-
bson_destroy(&opts);
865-
return false;
898+
goto cleanup;
866899
}
900+
901+
free_reply = true;
902+
867903
if (!result) {
868904
/* Server errors (other than ExceededTimeLimit) and write concern errors
869905
* may use CommandException and report the result document for the
@@ -888,22 +924,18 @@ int phongo_execute_command(mongoc_client_t* client, php_phongo_command_type_t ty
888924
} else {
889925
phongo_throw_exception_from_bson_error_t(&error TSRMLS_CC);
890926
}
891-
bson_destroy(&reply);
892-
bson_destroy(&opts);
893-
return false;
927+
goto cleanup;
894928
}
895929

896-
bson_destroy(&opts);
897-
898930
if (!return_value_used) {
899-
bson_destroy(&reply);
900-
return true;
931+
goto cleanup;
901932
}
902933

903934
/* According to mongoc_cursor_new_from_command_reply(), the reply bson_t
904935
* is ultimately destroyed on both success and failure. */
905936
if (bson_iter_init_find(&iter, &reply, "cursor") && BSON_ITER_HOLDS_DOCUMENT(&iter)) {
906937
bson_t initial_reply = BSON_INITIALIZER;
938+
bson_error_t error = {0};
907939

908940
bson_copy_to(&reply, &initial_reply);
909941

@@ -917,17 +949,39 @@ int phongo_execute_command(mongoc_client_t* client, php_phongo_command_type_t ty
917949
bson_append_int64(&initial_reply, "batchSize", -1, command->batch_size);
918950
}
919951

952+
if (zsession && !mongoc_client_session_append(Z_SESSION_OBJ_P(zsession)->client_session, &initial_reply, &error)) {
953+
phongo_throw_exception_from_bson_error_t(&error TSRMLS_CC);
954+
bson_destroy(&initial_reply);
955+
result = false;
956+
goto cleanup;
957+
}
958+
920959
cmd_cursor = mongoc_cursor_new_from_command_reply(client, &initial_reply, server_id);
921-
bson_destroy(&reply);
922960
} else {
923961
bson_t* wrapped_reply = create_wrapped_command_envelope(db, &reply);
924962

925963
cmd_cursor = mongoc_cursor_new_from_command_reply(client, wrapped_reply, server_id);
926-
bson_destroy(&reply);
927964
}
928965

929966
phongo_cursor_init_for_command(return_value, client, cmd_cursor, db, zcommand, zreadPreference, zsession TSRMLS_CC);
930-
return true;
967+
968+
cleanup:
969+
bson_destroy(&opts);
970+
971+
if (free_reply) {
972+
bson_destroy(&reply);
973+
}
974+
975+
if (free_zsession) {
976+
#if PHP_VERSION_ID >= 70000
977+
zval_ptr_dtor(zsession);
978+
efree(zsession);
979+
#else
980+
zval_ptr_dtor(&zsession);
981+
#endif
982+
}
983+
984+
return result;
931985
} /* }}} */
932986
/* }}} */
933987

‎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 mongoc_read_concern_t* re
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 = {
@@ -441,7 +461,7 @@ static HashTable* php_phongo_cursor_get_debug_info(zval* object, int* is_temp TS
441461
*is_temp = 1;
442462
intern = Z_CURSOR_OBJ_P(object);
443463

444-
array_init_size(&retval, 9);
464+
array_init_size(&retval, 10);
445465

446466
if (intern->database) {
447467
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)