Skip to content

Commit 200d0f8

Browse files
authored
CDRIVER-6112 fix ownership transfer of mongoc_write_command_t (#2132) (#2136)
* add regression test * do not memcpy `bson_t` struct in array * `memcpy` does not correctly transfer ownership of `bson_t`. Instead: heap allocate `bson_t`. * warn against using `bson_t` in `mongoc_array_t`
1 parent 6e2ee94 commit 200d0f8

File tree

4 files changed

+63
-5
lines changed

4 files changed

+63
-5
lines changed

src/libmongoc/src/mongoc/mongoc-array-private.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@
2525
BSON_BEGIN_DECLS
2626

2727

28+
// mongoc_array_t stores an array of objects of type T.
29+
//
30+
// T must be trivially relocatable. In particular, `bson_t` is not trivially relocatable (CDRIVER-6113).
2831
typedef struct _mongoc_array_t mongoc_array_t;
2932

3033

src/libmongoc/src/mongoc/mongoc-write-command-private.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ typedef struct {
6262
uint32_t n_documents;
6363
mongoc_bulk_write_flags_t flags;
6464
int64_t operation_id;
65-
bson_t cmd_opts;
65+
bson_t *cmd_opts;
6666
} mongoc_write_command_t;
6767

6868

src/libmongoc/src/mongoc/mongoc-write-command.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -144,9 +144,9 @@ _mongoc_write_command_init_bulk (
144144
command->flags = flags;
145145
command->operation_id = operation_id;
146146
if (!bson_empty0 (opts)) {
147-
bson_copy_to (opts, &command->cmd_opts);
147+
command->cmd_opts = bson_copy (opts);
148148
} else {
149-
bson_init (&command->cmd_opts);
149+
command->cmd_opts = bson_new ();
150150
}
151151

152152
_mongoc_buffer_init (&command->payload, NULL, 0, NULL, NULL);
@@ -668,7 +668,7 @@ _mongoc_write_opmsg (mongoc_write_command_t *command,
668668
? MONGOC_CMD_PARTS_ALLOW_TXN_NUMBER_NO
669669
: MONGOC_CMD_PARTS_ALLOW_TXN_NUMBER_YES;
670670

671-
BSON_ASSERT (bson_iter_init (&iter, &command->cmd_opts));
671+
BSON_ASSERT (bson_iter_init (&iter, command->cmd_opts));
672672
if (!mongoc_cmd_parts_append_opts (&parts, &iter, error)) {
673673
bson_destroy (&cmd);
674674
mongoc_cmd_parts_cleanup (&parts);
@@ -937,7 +937,7 @@ _mongoc_write_command_destroy (mongoc_write_command_t *command)
937937
ENTRY;
938938

939939
if (command) {
940-
bson_destroy (&command->cmd_opts);
940+
bson_destroy (command->cmd_opts);
941941
_mongoc_buffer_destroy (&command->payload);
942942
}
943943

src/libmongoc/tests/test-mongoc-bulk.c

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4807,6 +4807,54 @@ test_multiple_execution (void)
48074807
mongoc_client_destroy (client);
48084808
}
48094809

4810+
// `test_bulk_big_let` tests a bulk operation with a large let document to reproduce CDRIVER-6112:
4811+
static void
4812+
test_bulk_big_let (void *unused)
4813+
{
4814+
BSON_UNUSED (unused);
4815+
4816+
mongoc_client_t *client = test_framework_new_default_client ();
4817+
mongoc_collection_t *coll = get_test_collection (client, "test_big_let");
4818+
bson_error_t error;
4819+
4820+
// Create bulk operation similar to PHP driver:
4821+
mongoc_bulk_operation_t *bulk = mongoc_bulk_operation_new (true /* ordered */);
4822+
4823+
// Set a large `let`: { "testDocument": { "a": "aaa..." } }
4824+
{
4825+
bson_t let = BSON_INITIALIZER, testDocument;
4826+
bson_append_document_begin (&let, "testDocument", -1, &testDocument);
4827+
4828+
// Append big string:
4829+
{
4830+
size_t num_chars = 79;
4831+
char *big_string = bson_malloc0 (num_chars + 1);
4832+
memset (big_string, 'a', num_chars);
4833+
BSON_APPEND_UTF8 (&testDocument, "a", big_string);
4834+
bson_free (big_string);
4835+
}
4836+
4837+
bson_append_document_end (&let, &testDocument);
4838+
mongoc_bulk_operation_set_let (bulk, &let);
4839+
bson_destroy (&let);
4840+
}
4841+
4842+
4843+
mongoc_bulk_operation_set_client (bulk, client);
4844+
mongoc_bulk_operation_set_database (bulk, "db");
4845+
mongoc_bulk_operation_set_collection (bulk, "coll");
4846+
4847+
mongoc_bulk_operation_update (
4848+
bulk, tmp_bson ("{'_id': 1}"), tmp_bson ("{'$set': {'document': '$$testDocument'}}"), true);
4849+
4850+
4851+
ASSERT_OR_PRINT (mongoc_bulk_operation_execute (bulk, NULL, &error), error);
4852+
4853+
mongoc_bulk_operation_destroy (bulk);
4854+
mongoc_collection_destroy (coll);
4855+
mongoc_client_destroy (client);
4856+
}
4857+
48104858

48114859
void
48124860
test_bulk_install (TestSuite *suite)
@@ -4985,4 +5033,11 @@ test_bulk_install (TestSuite *suite)
49855033
"/BulkOperation/set_client_updates_operation_id_when_client_changes",
49865034
test_bulk_write_set_client_updates_operation_id_when_client_changes);
49875035
TestSuite_AddLive (suite, "/BulkOperation/multiple_execution", test_multiple_execution);
5036+
TestSuite_AddFull (
5037+
suite,
5038+
"/BulkOperation/big_let",
5039+
test_bulk_big_let,
5040+
NULL,
5041+
NULL,
5042+
test_framework_skip_if_max_wire_version_less_than_13 /* 5.0+ for 'let' support in CRUD commands */);
49885043
}

0 commit comments

Comments
 (0)