[CDRIVER-812] Memory Leak When Using bson_reinit() Created: 26/Aug/15  Updated: 19/Oct/16  Resolved: 09/Sep/15

Status: Closed
Project: C Driver
Component/s: docs, libbson
Affects Version/s: 1.1.10
Fix Version/s: 1.2.0

Type: Improvement Priority: Minor - P4
Reporter: Peng Xie Assignee: A. Jesse Jiryu Davis
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Related
is related to CDRIVER-811 Clearer Design & document on Invalid ... Closed

 Description   

The document said:

The bson_reinit() function shall be equivalent to calling bson_destroy() and bson_init().

But this demo shows their difference:

test.cc

#include <cassert>
#include <cstring>
 
#include <mongoc.h>
 
void log_bson(const bson_t *bson) {
  char *json = bson_as_json(bson, NULL);
  puts(json ? json : "[bson_as_json_error]");
 if (json) bson_free(json);
}
 
void fill(bson_t *p1, bson_t *p2) {
  bool status;
  status = bson_append_document_begin(p1, "$set", 4, p2);
  assert(status);
  status = bson_append_bool(p2, "foo", 3, true);
  assert(status);
  status = bson_append_document_end(p1, p2);
  assert(status);
}
 
#define USE_BSON_REINIT 1
 
int main() {
  bson_t bson = BSON_INITIALIZER;
  bson_t child = BSON_INITIALIZER;
  fill(&bson, &child);
  log_bson(&bson);
 
#if USE_BSON_REINIT
  bson_reinit(&bson);
#else
  bson_destroy(&bson);
  bson_init(&bson);
#endif
 
  bson_destroy(&child);
  bson_init(&child);
  fill(&child, &bson);
  log_bson(&child);
  bson_destroy(&bson);
  bson_destroy(&child);
}

Although outputs are the same when enable and disable USE_BSON_REINIT, valgrind detects a memory leak when USE_BSON_REINIT is true. Something like:

==7558== 128 bytes in 1 blocks are definitely lost in loss record 607 of 663
==7558==    at 0x4C2B0AF: malloc (vg_replace_malloc.c:296)
==7558==    by 0x598260B: bson_malloc (in /usr/lib64/libbson-1.0.so.0.0.0)
==7558==    by 0x5976EEC: _bson_grow (in /usr/lib64/libbson-1.0.so.0.0.0)
==7558==    by 0x5978307: _bson_append_bson_begin (in /usr/lib64/libbson-1.0.so.0.0.0)
==7558==    by 0x108C69: fill(_bson_t*, _bson_t*) (test.cc:14)
==7558==    by 0x108DD1: main (test.cc:27)



 Comments   
Comment by Githook User [ 11/Jan/16 ]

Author:

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

Message: Merge branch '1.2.0-dev'

Comment by Githook User [ 11/Jan/16 ]

Author:

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

Message: CDRIVER-812 notes about bson_t initialization
Branch: 1.3.0-dev
https://github.com/mongodb/libbson/commit/dec3b720a5dc3d7a375f779b697528de2e511916

Comment by Githook User [ 20/Oct/15 ]

Author:

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

Message: Merge branch '1.2.0-dev'

Comment by Githook User [ 20/Oct/15 ]

Author:

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

Message: CDRIVER-812 notes about bson_t initialization
Branch: debian
https://github.com/mongodb/libbson/commit/dec3b720a5dc3d7a375f779b697528de2e511916

Comment by Githook User [ 07/Oct/15 ]

Author:

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

Message: Merge branch '1.2.0-dev'

Comment by Githook User [ 07/Oct/15 ]

Author:

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

Message: CDRIVER-812 notes about bson_t initialization
Branch: master
https://github.com/mongodb/libbson/commit/dec3b720a5dc3d7a375f779b697528de2e511916

Comment by Githook User [ 07/Oct/15 ]

Author:

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

Message: Merge branch '1.2.0-dev'

Comment by Githook User [ 09/Sep/15 ]

Author:

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

Message: CDRIVER-812 notes about bson_t initialization
Branch: 1.2.0-dev
https://github.com/mongodb/libbson/commit/dec3b720a5dc3d7a375f779b697528de2e511916

Comment by A. Jesse Jiryu Davis [ 28/Aug/15 ]

bson_copy_to has the right kind of guidance that bson_append_document_begin needs:

http://api.mongodb.org/libbson/current/bson_copy_to.html

Comment by A. Jesse Jiryu Davis [ 27/Aug/15 ]

To resolve this ticket, I'm going to update the docs for bson_reinit and for bson_append_document_begin. The bson_reinit will describe the stack vs. malloc'ed buffer distinction and note that bson_reinit doesn't free the buffer; you still must call bson_destroy to free it.

If the API is used correctly then the current documentation is still accurate:

The bson_reinit() function shall be equivalent to calling bson_destroy() and bson_init().

However, if the bson_t structure contains a malloc()'d buffer, it may be reused.

... but a more thorough note might've helped you understand the cause of your leak.

I'll update the doc for bson_append_document_begin. Currently it just says:

child: A bson_t to be initialized as the sub-document.

... but that could much more prominently warn that "child" must not be initialized before passing it to bson_append_document_begin.

Comment by A. Jesse Jiryu Davis [ 27/Aug/15 ]

I can understand your point that the documentation is incomplete. Here's what happens:

  1. You initialized the bson_t object "bson". At first, it has no malloc'ed buffer, it only has a small internal buffer that's included in the object itself on the stack
  2. You add enough fields to "bson" that it outgrows its internal buffer and mallocs a buffer for itself on the heap
  3. You call bson_reinit on "bson", which clears its fields but does not waste time freeing the malloc'ed buffer. Instead it keeps that buffer available to add new fields to, until you call bson_destroy on "bson".
  4. Then you call bson_append_document_begin with "bson" as the second argument (the argument named "child"). bson_append_document_begin expects an uninitialized bson_t as the second argument, so it overwrites its internal structure without checking if there's a malloc'ed buffer to free.
  5. Thus the malloc'ed buffer is forgotten and leaked, even if you call bson_destroy on "bson" at the end.

On the other hand, if in step 3 you destroy and init "bson" instead of bson_reinit, that actually spends the time freeing the malloc'ed buffer and reinitializing "bson" with the stack-based internal buffer, just as at the beginning.

It's still technically incorrect to pass an initialized bson_t as the second argument to bson_append_document_begin, but in this second scenario it doesn't cause a leak because the bson_t has no malloc'ed buffer any more.

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