PHPC-1151: Maintain Session reference on Cursor #801
Conversation
tests/cursor/bug1151-001.phpt
Outdated
| --SKIPIF-- | ||
| <?php require __DIR__ . "/../utils/basic-skipif.inc"; ?> | ||
| <?php NEEDS_CRYPTO(); ?> | ||
| <?php NEEDS('STANDALONE'); NEEDS_ATLEAST_MONGODB_VERSION(STANDALONE, "3.6"); ?> |
There was a problem hiding this comment.
Previous build failure was due to Travis attempting to run these tests on a pre-3.6 server. This adds the necessary skips from other session tests; however, it means that these tests won't run on Travis at all.
This would probably be a good time to bring over the manual server installation we're currently doing in PHPLIB.
| * assigned to the option. On error, false is returned and an exception is | ||
| * thrown. */ | ||
| static bool phongo_parse_session(zval *options, bson_t *mongoc_opts, zval **zsession, mongoc_client_t *client TSRMLS_DC) /* {{{ */ | ||
| static bool phongo_parse_session(zval *options, mongoc_client_t *client, bson_t *mongoc_opts, zval **zsession TSRMLS_DC) /* {{{ */ |
There was a problem hiding this comment.
What's the reason for moving the argument order around?
There was a problem hiding this comment.
See commit message: 0b6f04f
| @@ -54,6 +54,8 @@ object(MongoDB\Driver\Cursor)#%d (%d) { | |||
| } | |||
| ["readPreference"]=> | |||
| NULL | |||
| ["session"]=> | |||
| NULL | |||
There was a problem hiding this comment.
Do we have a test case where "session" is not NULL?
There was a problem hiding this comment.
I think putting it in #803 makes more sense.
| @@ -54,6 +54,8 @@ object(MongoDB\Driver\Cursor)#%d (%d) { | |||
| } | |||
| ["readPreference"]=> | |||
| NULL | |||
| ["session"]=> | |||
| NULL | |||
There was a problem hiding this comment.
I think putting it in #803 makes more sense.
Client is required, while the opts and zval arguments are optional.
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.
https://jira.mongodb.org/browse/PHPC-1151
Although Cursors no longer store a reference to client objects, this is similar to what is still done for ReadPreference objects (see: 78839bd).