PHPC-1700: Fix memory leak when read preference is invalid

[alcaeus]

PHPC-1700

This fixes a memory leak in php_phongo_readpreference_init_from_hash that is caused by not properly separating zvals before modifying read preference tags. The issue can be observed when MongoDB\Driver\ReadPreference::__set_state errors after preparing tag sets (e.g. because tags were combined with a primary mode). Contrary to the original issue, this problem does not affect other instances where the underlying logic from php_phongo_read_preference_prep_tagsets is used, as the same test (tags with primary read preference) does not leak in the ReadPreference constructor.

This PR also adds a regression test for PHPC-1701 that also assumed the same issue would apply when preparing properties for authentication mechanisms. This test does not leak since all involved zvals are separated, but the test is included to prevent regressions.

[alcaeus]

Keeping with the tradition of using ZVAL_DEREF and SEPARATE_ZVAL_NOREF. If it's more sensible to use SEPARATE_ZVAL here (and in other cases where we separate), I'd like to create a separate ticket for this.

[jmikola]

Does this address the following comment from PHPC-1700?

Additionally, Manager may also leak if an exception occurs after preparing the readPreferenceTags key in the URI options array. That may be possible if merge_context_options or phongo_manager_init throws.

The fix here is only for php_phongo_readpreference_init_from_hash, which Manager doesn't use.

[alcaeus]

Manager does not leak in that case. manager-ctor-read_prefernece-error-001.phpt already checks for this by attempting to create a manager by combining primary read preference with tags. That test did not leak when I ran it with memcheck.

The leak does not originate from throwing an exception after preparing tagsets, but from the missing zval separation in __set_state. This is also evidenced by the change necessary to fix bug1698-001.phpt, which started failing after the fix. This is because the expectation there is for the tags to be an object after calling __set_state, which shouldn't happen if we separated the zval.

[jmikola]

One question about Manager's handling of tagsets.

Apart from that, I ran all of the readPreference/ and manager/ tests that use "readPreferenceTags" with Valgrind and observed on leaks; however, I didn't create a separate test where the Manager constructor errors out after preparing tag sets.

[jmikola]

Thanks for the explanation about set_state. That makes more sense.