[CDRIVER-446] Memory leak in mongoc_client_command_simple() Created: 19/Oct/14  Updated: 19/Oct/16  Resolved: 05/Nov/14

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

Type: Bug Priority: Major - P3
Reporter: Jerome Lebel Assignee: Mira Carey
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified


 Description   

when I call mongoc_database_get_collection_info() a new bson is created:
https://github.com/mongodb/mongo-c-driver/blob/048c851919dc32c98b5f8a9a0270e69af9385db3/src/mongoc/mongoc-database.c#L741

the bson correct. But, this bson is sent to mongoc_database_command_simple() and then to mongoc_client_command_simple()

The issue is this bson is sent to bson_init() :
https://github.com/mongodb/mongo-c-driver/blob/048c851919dc32c98b5f8a9a0270e69af9385db3/src/mongoc/mongoc-client.c#L1247

so the flags of this bson changes from BSON_FLAG_INLINE to BSON_FLAG_INLINE | BSON_FLAG_STATIC

So later when bson_destroy() is called, the buffer is not freed()

I think bson_reinit() should be called instead. But I'm not sure. I'm not even sure if we need to call something on this bson.



 Comments   
Comment by Jerome Lebel [ 06/Nov/14 ]

My idea was to say all bson from reply as to be initialized. That we can either do:
bson command_reply = BSON_INITIALIZER;
mongoc_database_command_simple(database, &cmd, read_prefs, &command_reply, error);

bson *command_reply = bson_new();
mongoc_database_command_simple(database, &cmd, read_prefs, &command_reply, error);

for me this should not be allowed:
bson command_reply;

only this should exist:
bson command_reply = BSON_INITIALIZER;

But well that's my personal opinion, and as long as I don't have memory leaks, I'm happy.

Thanks!

Comment by Jerome Lebel [ 06/Nov/14 ]

Sorry, I made a mistake between branches. I tested a branch that doesn't have your fix yet. I tried the master, I don't see the leak anymore!

Thanks.

Comment by Mira Carey [ 06/Nov/14 ]

I'm not sure where you're seeing that leak. I've been making sure to run the test suite through valgrind and confirmed the leak you were seeing, fixed it, and then observed that valgrind no longer reported the leak. If there's a use case I've missed I'd be happy to add it to the test suite and reopen this.

I'm sorry if you're not thrilled with the way I fixed this, but I had a couple of constraints I tried to work within:

  • mongoc_client_command_simple() expects an uninitialized stack allocated reply. That's not as specific as it should have been, and probably justifies some more explicit documentation, but that's the contract at this point. I don't really want to break users who have figured that out.
  • There's no way to identify an uninitialized stack allocation. We don't demand that users zero bson_t's before passing them through libmongoc api, or that they use the static initializer, so we can't rely on any kind of bit pattern to identify bson_t's with an existing heap allocation.
  • I can work around that by providing a stack allocation and then copying it into a heap allocated reply and passing that back. Which is the fix here. It's not ideal, but it works around an otherwise unavoidable memory leak in mongoc_database_get_collection_info()

I apologize if I'm missing something obvious, but I don't see how database_get_collection_info leaks at this point in master

Comment by Jerome Lebel [ 05/Nov/14 ]

There is still a memory leak in mongoc_client_command_simple() when using mongoc_database_get_collection_info()

Comment by Jerome Lebel [ 04/Nov/14 ]

I don't really understand why it is fixed. bson_init() is still called, therefore the flags are still changed. Anyone who doesn't know this issue will create a memory leak.

For me the consequences are fixed, but not the origin of this bug.

Comment by Githook User [ 04/Nov/14 ]

Author:

{u'username': u'hanumantmk', u'name': u'Jason Carey (hanumantmk)', u'email': u'jcarey@argv.me'}

Message: CDRIVER-446 Fix a leak in mongoc_client_command_simple()

fix a leak in mongoc_client_command_simple().

Fix some other random leaks while we're at it

Closes #110
Branch: master
https://github.com/mongodb/mongo-c-driver/commit/021a050ef5705515acb7e391b0bfe9950f17cee1

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