[CDRIVER-2341] Segfault destroying client if one of its sessions is not destroyed first Created: 03/Nov/17  Updated: 28/Oct/23  Resolved: 03/Nov/17

Status: Closed
Project: C Driver
Component/s: libmongoc
Affects Version/s: None
Fix Version/s: None

Type: Bug Priority: Major - P3
Reporter: Jeremy Mikola Assignee: A. Jesse Jiryu Davis
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Related
is related to CDRIVER-2192 Implement Driver Sessions API Closed
is related to CDRIVER-2228 All writes retryable support Closed

 Description   

While implementing retryable write tests (CDRIVER-2228), I encountered a segfault when I forgot to destroy a session:

mongoc_uri_t *uri;
mongoc_client_t *client;
mongoc_client_session_t *session;
mongoc_collection_t *collection;
bson_t test;
bson_error_t error;
 
uri = test_framework_get_uri ();
mongoc_uri_set_option_as_bool (uri, "retryWrites", false);
 
client = mongoc_client_new_from_uri (uri);
test_framework_set_ssl_opts (client);
mongoc_uri_destroy (uri);
 
/* TODO: Remove this once start_session initializes the topology */
mongoc_client_command_simple (client, "admin", tmp_bson ("{'ping': 1}"), NULL, NULL, &error);
 
session = mongoc_client_start_session (client, NULL, &error);
 
collection = get_test_collection (client, "retryable_writes");
 
// Commenting these out, but these are functions that run the JSON test spec
// insert_data (collection, scenario);
// execute_test (collection, &test, session);
 
if (!mongoc_collection_drop (collection, &error)) {
   if (strcmp (error.message, "ns not found")) {
      /* an error besides ns not found */
      ASSERT_OR_PRINT (false, error);
   }
}
 
mongoc_collection_destroy (collection);
// If the session is not destroyed at all, we get a segfault during client destruction
// mongoc_client_session_destroy (session);
mongoc_client_destroy (client);

Backtrace:

#0  0x0000000000000000 in ?? ()
#1  0x0000000000502c2c in mongoc_set_for_each (set=0x502c2c <mongoc_set_for_each+38>, cb=0x7ffd52361f50, ctx=0x7fe8bdd4557d <bson_free+28>) at src/mongoc/mongoc-set.c:163
#2  0x00000000004d1373 in mongoc_client_get_uri (client=0x27a8260) at src/mongoc/mongoc-client.c:1059
#3  0x000000000048f33e in test_all_spec_tests (suite=0x561707) at tests/test-mongoc-retryable-writes.c:656
#4  0x00000000004c3b95 in TestSuite_RunTest (suite=0x7ffd523627b0, test=0x275b1b0, count=0x7ffd52362738) at tests/TestSuite.c:698
#5  0x00000000004c463d in TestSuite_RunNamed (suite=0x7ffd523627b0, testname=0x2736270 "/retryable_writes/*") at tests/TestSuite.c:1021
#6  0x00000000004c4767 in TestSuite_Destroy (suite=0x7ffd523627b0) at tests/TestSuite.c:1061
#7  0x0000000000419a45 in main (argc=5, argv=0x7ffd523628f8) at tests/test-libmongoc.c:2147



 Comments   
Comment by A. Jesse Jiryu Davis [ 03/Nov/17 ]

The client keeps a mongoc_set_t of active mongoc_client_session_t's. Normally a mongoc_set_t also has a "dtor" field, which is a function pointer to an item destructor, but in the case of this set there is no destructor so the function pointer is NULL.

This isn't a problem if all sessions are already removed from the set by mongoc_client_session_destroy before the client is destroyed. But if a session is leaked, then the set is non-empty and mongoc_client_session_destroy tries to call the destructor, and executes a NULL function pointer.

I've updated the mongoc_set_t to not call the "dtor" function if it's NULL, and clarified the docs to say that a session must be destroyed before the client it came from.

Comment by Githook User [ 03/Nov/17 ]

Author:

{'name': 'A. Jesse Jiryu Davis', 'username': 'ajdavis', 'email': 'jesse@mongodb.com'}

Message: CDRIVER-2341 clarify session lifecycle
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/222d41277a3075313998390a681e357c169b78b3

Comment by Githook User [ 03/Nov/17 ]

Author:

{'name': 'A. Jesse Jiryu Davis', 'username': 'ajdavis', 'email': 'jesse@mongodb.com'}

Message: CDRIVER-2341 crash in mongoc_set_destroy

The set-item destructor should be optional. Don't call it if it's NULL.
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/24bb13123d411b6f77f9be7a298f085ac7f7cc71

Generated at Wed Feb 07 21:14:56 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.