PHPC-1230: Support object cloning for BSON classes

[alcaeus]

https://jira.mongodb.org/browse/PHPC-1230

In most cases, I've opted to initialise the cloned object using the php_phongo_*_init functions. One of the few exceptions is MongoDB\BSON\Decimal128, where we copy the bson_decimal128_t struct via memcpy to avoid converting back and forth.

From the ticket:

This should be a relatively straightforward task, since only Javascript contains a non-scalar property. Even then, the internal properties of each BSON class are immutable, so a deep copy may not be required there; however, we should test Javascript scopes thoroughly.

I've opted to use bson_new and bson_copy_to for the scope. This should properly copy the scope for the cloned object so we're not running into any issues.

[jmikola]

Documentation for bson_copy_to() states that "dst must be an uninitialized bson_t to avoid leaking memory." If you run the corresponding Javascript clone test with TEST_PHP_ARGS=-m, does this come up as a leak?

[alcaeus]

I have to set up proper non-MacOS tooling as valgrind still doesn't work with MacOS 😔

I believe the issue with bson_copy_to is that it doesn't free previous buffer, leading to memory leaks. Since we never have a buffer but only create an empty BSON struct, there is no memory to be leaked here.

FWIW, removing the bson_new call above leads to a segmentation fault.

[jmikola]

I can test this locally and get back to you.

[jmikola]

There is a definitely a leak with the above code:

128 bytes in 1 blocks are definitely lost in loss record 25 of 30
   at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
   by 0x7E9251: __zend_malloc (zend_alloc.c:2829)
   by 0xE58C2E7: php_phongo_malloc (php_phongo.c:2743)
   by 0xE5018B4: bson_malloc (bson-memory.c:70)
   by 0xE4EF2F7: bson_new (bson.c:2039)
   by 0xE5987BD: php_phongo_javascript_clone_object (Javascript.c:440)
   by 0x8D586D: ZEND_CLONE_SPEC_CV_HANDLER (zend_vm_execute.h:33778)
   by 0x911AC6: execute_ex (zend_vm_execute.h:61991)
   by 0x91402D: zend_execute (zend_vm_execute.h:63776)
   by 0x8239AE: zend_execute_scripts (zend.c:1502)
   by 0x787656: php_execute_script (main.c:2590)
   by 0x916CD9: do_cli (php_cli.c:1011)

FWIW, removing the bson_new call above leads to a segmentation fault.

This appears to be due to the BSON_ASSERT(dst). I observed the following error when running the clone test with bson_new removed:

src/libmongoc/src/libbson/src/bson/bson.c:2194 bson_copy_to(): precondition failed: dst
Aborted (core dumped)

It appears bson_copy_to() expects to receive a pointer to an uninitialized bson_t struct. In practice, that would be the address of a stack-allocated bson_t, rather than something returned by bson_new(). Alternatively, you could calloc a block of memory with the size of a bson_t, but that's unconventional.

bson_copy() seems more appropriate here:

new_intern->scope = bson_copy(intern->scope);

[alcaeus]

Ah, bson_copy sounds like it does what I was looking for. I saw bson_copy_to being used in a few places and got confused 😕

[jmikola]

Optional suggestion for hard-coding ObjectId lengths, but LGTM with or without that.

[jmikola]

Replace #001 with "can be cloned"

[jmikola]

Note: UTCDateTime test has some copypasta in its description. LGTM otherwise.