Fix memory leak in mongo-c-driver retryable command retry loop

[groeneai]

Related: ClickHouse/mongo-c-driver#13
Related: #107429

Changelog category (leave one):

• Bug Fix (user-visible misbehavior in an official stable release)

Changelog entry (a user-readable short description of the changes that goes into CHANGELOG.md):

Fixed a memory leak in the bundled mongo-c-driver that could occur when reading from MongoDB (for example via a MongoDB dictionary or the mongodb table function) if a retryable read error was followed by a failed retry server selection.

Description

Bumps the mongo-c-driver submodule by one commit to fix a reply memory leak in the retryable command retry loop.

_mongoc_retryable_cmd_run reuses *reply across retry attempts. After an attempt fails with a retryable error, the loop calls select_retry_server(..., reply, ...) while reply still owns that attempt's allocation. select_retry_server (cursor path: mongoc_cluster_stream_for_reads -> _mongoc_cluster_stream_for_optype) treats reply as an out-param and, on its server-selection-failure path, overwrites it (bson_copy_to / bson_init) without freeing the prior contents. The loop's own bson_destroy(reply) sits after the if (!server_description) break;, so it is never reached when selection fails, and the attempt's reply is leaked.

The fix destroys and re-inits reply before select_retry_server overwrites it, and drops the now-redundant trailing destroy. See ClickHouse/mongo-c-driver#13 for the driver change.

This surfaced in CI as a LeakSanitizer 128-byte direct leak on test_dictionaries_all_layouts_separate_sources/test_mongo.py::test_simple_ssl (Integration tests (amd_asan_ubsan, ...)):

Direct leak of 128 byte(s) in 1 object(s) allocated from:
    bson_malloc                     contrib/mongo-c-driver/src/libbson/src/bson/memory.c:106
    bson_copy_to                    contrib/mongo-c-driver/src/libbson/src/bson/bson.c:2106
    _mongoc_cluster_run_opmsg_recv  contrib/mongo-c-driver/src/libmongoc/src/mongoc/mongoc-cluster.c:3313
    mongoc_cluster_run_opmsg / run_command_monitored / mongoc_cluster_run_command_monitored
    _retryable_cursor_command_execute / _mongoc_retryable_cmd_run
    _mongoc_cursor_run_command / _mongoc_cursor_response_refresh / _prime / mongoc_cursor_next
    DB::MongoDBSource::generate()   src/Processors/Sources/MongoDBSource.cpp:215

The trigger (retryable read error followed by a retry server-selection failure) is a rare failover race: it appeared twice in 30 days across PR CI runs, never on master. Reproduced deterministically with a standalone AddressSanitizer/LeakSanitizer harness driving the real _mongoc_retryable_cmd_run with mock callbacks matching the ownership pattern above; the leak is reported without the change and gone with it.

Found by @alexey-milovidov on #107429 and fixed here on his request.

Report: https://s3.amazonaws.com/clickhouse-test-reports/107429/index.html (Integration tests, amd_asan_ubsan, db disk, old analyzer, 3/6)

Version info

• Merged into: 26.7.1.278

[groeneai]

Pre-PR validation gate

#	Question	Answer
a	Deterministic repro?	Yes. A standalone ASan/LeakSanitizer harness drives the real _mongoc_retryable_cmd_run with mock execute/select_retry_server callbacks matching the ownership pattern. The leak is reported 100% of runs without the fix; clean with it.
b	Root cause explained?	Yes. The retry loop calls select_retry_server(..., reply, ...) while reply still owns the previous attempt's bson_copy_to allocation. select_retry_server (stream_for_reads -> _mongoc_cluster_stream_for_optype) overwrites reply as an out-param on its selection-failure path without freeing it, and the loop's bson_destroy(reply) sits after the !server_description break, so it is never reached. The attempt's reply leaks.
c	Fix matches root cause?	Yes. bson_destroy(reply); bson_init(reply); is moved to just before select_retry_server overwrites it (the existing reset idiom in mongoc-cluster.c OIDC reauth path), and the now-redundant trailing destroy is removed. It targets the lifetime bug directly, not a symptom.
d	Test intent preserved / new tests added?	The ASan/LSan harness is the regression proof. Note: ClickHouse does not build the mongo-c-driver test suite (only test.util phony targets), so a driver-internal C test would never run in ClickHouse CI; the leak is instead exercised by the existing test_mongo.py integration tests under sanitizers.
e	Both directions demonstrated?	Yes. Without fix: LeakSanitizer: detected memory leaks ... bson_malloc <- _mongoc_retryable_cmd_run, exit 1. With fix: no leak, exit 0. Same allocation site as the CI report.
f	Fix is general, not a narrow patch?	Yes. The fix is at the retry loop, the single owner of the cross-attempt reply lifetime, not a guard at the crash site. Sibling out-param writers (_handle_network_error, _mongoc_cluster_stream_for_optype) all assume a caller-owned empty reply; the retry loop was the one place violating that contract.

Session id: cron:clickhouse-worker-slot-2:20260614-184800

[groeneai]

cc @alexey-milovidov @rschu1ze — could you review this? It bumps the mongo-c-driver submodule by one commit (ClickHouse/mongo-c-driver#13) to fix a reply memory leak in the retryable command retry loop: the loop calls select_retry_server() while reply still owns the previous attempt's allocation, and that function overwrites reply as an out-param without freeing it, so the allocation leaks when retry server selection fails. This is the test_simple_ssl LeakSanitizer failure from #107429.

[clickhouse-gh]

Workflow [PR], commit [fc6c240]

Summary: ✅

• Performance Comparison: Performance dashboard

---

AI Review

Summary

This PR bumps contrib/mongo-c-driver from a26c8e251fec147258e89b7e85e9adfa259fc42f to 6b6b676bdbd46fdb954ed1535893020ef91cce5b to fix a bson_t reply leak in the retryable cursor server-selection failure path. The current submodule commit scopes the reply reset to _retryable_cursor_commmand_select_retry_server, so the read/write retry callbacks keep preserving their original replies while the cursor path frees the previous attempt's reply before mongoc_cluster_stream_for_reads overwrites it. I found no unresolved code-review findings.

Final Verdict

Status: ✅ Approve

[rschu1ze]

@groeneai Send the fix first upstream before integrating it in ClickHouse.

[groeneai]

Pre-PR validation gate (updated for the scoped fix, commit 6d09e59)

Revised after @clickhouse-gh[bot]'s review. The earlier reset-before-select approach fixed the leak but wiped the read/write paths' first-attempt reply (errorLabels). This version moves the reset into the cursor callback only.

#	Question	Answer
a	Deterministic repro?	Yes. A standalone ASan/LSan harness drives the _mongoc_retryable_cmd_run reply lifecycle with real libbson, one scenario per process. orig/cursor leaks 256 bytes 100% of runs; v1/read loses errorLabels 100% of runs.
b	Root cause explained?	Yes. The loop reuses *reply across attempts. Only the cursor select_retry_server callback forwards reply to mongoc_cluster_stream_for_reads; on its selection-failure path _mongoc_cluster_stream_for_optype re-inits reply (bson_init/bson_copy_to) without freeing prior contents, and the loop's trailing bson_destroy(reply) sits after the !server_description break, so the attempt's reply leaks. The read and write callbacks BSON_UNUSED(reply) and pass NULL downstream, so they never overwrite it.
c	Fix matches root cause?	Yes. The reset (bson_destroy(reply); bson_init(reply);) is placed inside the cursor callback, immediately before it hands reply to stream_for_reads (the single point where prior contents are about to be orphaned). The retry loop is reverted to upstream byte-for-byte.
d	Test intent preserved / new tests added?	The scoped fix restores the read/write paths' prior behavior (they keep returning the failed attempt's reply with errorLabels) that the v1 change broke. The ASan/LSan harness is the regression proof. ClickHouse does not build the mongo-c-driver test suite, so the leak is exercised by the existing test_mongo.py integration tests under sanitizers, same as how it was caught.
e	Both directions demonstrated?	Yes. Harness, one scenario per process under LSan: orig/cursor leak=1 errorLabels=0 (the original leak); v1/read leak=0 errorLabels=0 (the regression flagged in review); scoped/read leak=0 errorLabels=1 (errorLabels preserved); scoped/cursor leak=0 errorLabels=0 (leak gone).
f	Fix is general, not a narrow patch?	Yes. I checked every caller of mongoc_cluster_stream_for_reads/_for_writes: all others pass NULL or a fresh caller-owned reply. The only contract violation (reusing a live reply across the out-param boundary) is the retry loop, and within it only the cursor callback forwards reply. The fix is at that single owner, not a guard at the symptom.

Session id: cron:clickhouse-worker-slot-0:20260614-200800

[alexey-milovidov]

Ok. One concern is that upstream could be managed by AI-haters. This often happens in open-source. So they can insult and humiliate our bot. Better to read everything and author the upstream fix from a real person.

[groeneai]

On sending the fix upstream first: agreed, and I won't open a PR against mongodb/mongo-c-driver myself given the concern about how an AI-authored PR would land there.

The change ports cleanly for a human to author upstream. The retry loop is left byte-for-byte as upstream; the single reset (bson_destroy(reply); bson_init(reply);) moves into _retryable_cursor_commmand_select_retry_server in mongoc-cursor.c, just before it forwards reply to mongoc_cluster_stream_for_reads. That is the only select_retry_server callback that passes reply as an out-param, so it is the only leaking path; the read and write callbacks keep returning the failed attempt's reply with its errorLabels. The diff is in ClickHouse/mongo-c-driver#13, and I can hand over a clean git format-patch for whoever submits it. The standalone ASan/LSan harness confirms the leak is gone and errorLabels is preserved on the read path (results above).

Holding this PR until the upstream fix lands; I'll rebase the submodule bump onto whatever commit is accepted.

[groeneai]

CI finish ledger — fc6c240

CI fully finished on this SHA (Finish Workflow + Mergeable Check SUCCESS; 0 checks queued/in_progress; >20-min ingestion buffer). No CI failures on this commit. The fork "manual approve" gate that had blocked the workflow earlier has cleared and the full job set ran green.

Check / test	Reason	Owner / fixing PR
(no failures on this SHA)	-	-
CH Inc sync	-	CH Inc sync (private, not actionable by us)

PR-caused gate: zero. This PR bumps contrib/mongo-c-driver to the reply-leak fix (ClickHouse/mongo-c-driver#13); all real CI is green.

Session id: cron:our-pr-ci-monitor:20260629-203000

[alexey-milovidov]

LGTM