fix(matrix-client): free replaced matrix-sdk clients #158

Merged

jlxq0 merged 3 commits from fix/memory-leak-20261102 into main 2026-11-02 12:04:14 +00:00

Conversation 0 Commits 3 Files changed 4 +296 -2

jlxq0 commented 2026-11-02 11:20:22 +00:00

Owner

• Copy link
• Copy Markdown

Root cause

matrix-sdk (0.19.1 client/mod.rs:3630, and 0.17 the same) builds LatestEvents with SendQueue::new(self.clone()). That is a strong Client, and it is moved into listen_to_updates_task. The task's abort handle is stored in ClientInner::latest_events, so the task is only aborted when the very Client it keeps alive is dropped. It only ends on its own when one of its three broadcast channels closes, and all three belong to that same client. The SDK initialises LatestEvents inside process_sync whenever the event cache is subscribed. We subscribe it for latest_event (#64, May 2026).

So every client that completed a single sync was a reference cycle. It was never freed after a token change or an auth-expiry eviction replaced it. What stayed behind each time: its state, crypto and event-cache stores with their open SQLite pools, every room, and every event it had synced (the event cache never shrinks a room that has no subscriber). Production replaces a client each time the MAS access token expires (6 builds and 6 permanent auth failure exits in the 6 h of logs).

Evidence

Measured in-process against a wiremock homeserver. Heap is glibc mallinfo2 (in use + mmapped). A witness Arc captured by an event handler that ClientInner owns shows whether ClientInner was freed.

	before	after
client replaced 5x, each after a successful sync	1 ClientInner never freed, 24-33 store fds stay open after evict	all freed, 0 fds
50 rooms, 1,500 events, then evict, +15 s, mock dropped	8,510 KiB still held	794 KiB (control without the event cache: 763 KiB)

So the harness leaks about 7.7 MiB per client at 50 rooms and 1,500 events. To make 250 MiB/day at about 24 replacements a day, each leaked client needs about 10 MiB. A larger account with an hour of traffic fits that figure. This is an inference: I did not measure a production client.

A first version of the experiment had a mock that returned 404 for /versions. With that mock, no client ever reached /sync and every client was freed. The leak only shows up once the mock serves /versions and a key upload. AGENTS.md now records this.

Fix

Each cached client initialises LatestEvents itself, before its sync loop starts, on a one-worker tokio runtime that the entry owns (LatestEventsRuntime). The runtime is shut down (shutdown_background) when the entry is dropped. Tokio then drops the runtime's tasks, which breaks the cycle. latest_event behaviour is unchanged. The cost is one extra worker thread per live client.

Other candidates, quantified (not the cause, not changed)

• rate_limit per-bearer buckets (bearer_read, bearer_write, initialize bearer) are never pruned. That is about 250 B per entry per map, so about 750 B per distinct bearer. Even at one bearer every 5 min this is about 0.2 MiB/day.
• mas introspection cache: soft cap of 256 with a sweep of expired entries, TTL ≤ 60 s. Bounded.
• last_used: capped at 1024 entries.
• build_locks and retiring: one small entry per ClientKey. A Weak<Client> pins only the 24-byte outer allocation, not ClientInner.
• session.rs workers: removed on close_session and reaped when the worker transport drops.
• channel PeerMap, PendingMap and ConfigCache: keyed per route, with TTL and cap where it matters. Bounded by the number of routes.

Remaining growth to know about (not fixed here)

While a client is alive, its event cache keeps every synced event in memory. LatestEvents listens to generic updates, not through a RoomEventCacheSubscriber, so auto-shrink never fires. The harness shows about 1.9 KB per 200-byte text event. A client rebuilt hourly stays bounded. A long-lived client, such as a /channel session with a non-expiring token, grows by that amount per event it receives. Bounding this means periodically subscribing to and dropping each room's event cache, which triggers the SDK's own shrink. That is a separate decision.

SQLite SQLITE_IOERR_GETTEMPPATH (6410)

The app cannot fix this cleanly in-process. matrix-sdk-sqlite opens its own connections and exposes no pragma hook, so there is no temp_store = MEMORY. Setting SQLITE_TMPDIR from Rust needs std::env::set_var (unsafe in edition 2024) or the sqlite3_temp_directory FFI, and the crate has unsafe_code = "forbid". The deployment-side emptyDir on /tmp is the right fix. A no-code alternative is the env var SQLITE_TMPDIR=<store dir>, which puts temp files on the PVC.

Second commit: RUSTSEC-2026-0433

The first CI run passed fmt, clippy and all 393 tests, then failed cargo audit. The cause was RUSTSEC-2026-0433 (pulldown-cmark 0.13.5, quadratic wikilink parsing, high), which reached the advisory database today. The lockfile had not changed, so main is affected too. cargo update -p pulldown-cmark --precise 0.13.6 changes only the lockfile. The package is transitive via ruma-events markdown support and 0.13.6 declares rust-version 1.71.1. All gates were re-run after the bump.

Verification

• cargo +1.98.0 fmt --all --check: clean
• cargo +1.98.0 clippy --all-targets --all-features --locked -- -D warnings: clean
• RUSTFLAGS=-Dwarnings cargo +1.98.0 test --all-features --locked: 387 + 6 passed
• cargo +1.96.0 check --all-targets --all-features --locked: ok (build floor)
• crypto dependency floor unittest and check: ok
• cargo audit: 0 vulnerabilities, with the two existing allowed unmaintained notices. cargo deny check bans licenses sources: ok
• Mutation checks: removing the LatestEventsRuntime::start call, or keeping the runtime alive (mem::forget), each make a_replaced_client_that_has_synced_is_freed fail with "generation 0 was replaced but its client was never freed".

Not merged, tagged or deployed.

🤖 Generated with Claude Code

https://claude.ai/code/session_01E2xQRZDxzCiJjLb9uAhP9j

## Root cause matrix-sdk (0.19.1 `client/mod.rs:3630`, and 0.17 the same) builds `LatestEvents` with `SendQueue::new(self.clone())`. That is a strong `Client`, and it is moved into `listen_to_updates_task`. The task's abort handle is stored in `ClientInner::latest_events`, so the task is only aborted when the very `Client` it keeps alive is dropped. It only ends on its own when one of its three broadcast channels closes, and all three belong to that same client. The SDK initialises `LatestEvents` inside `process_sync` whenever the event cache is subscribed. We subscribe it for `latest_event` (#64, May 2026). So every client that completed a single sync was a reference cycle. It was never freed after a token change or an auth-expiry eviction replaced it. What stayed behind each time: its state, crypto and event-cache stores with their open SQLite pools, every room, and every event it had synced (the event cache never shrinks a room that has no subscriber). Production replaces a client each time the MAS access token expires (6 builds and 6 `permanent auth failure` exits in the 6 h of logs). ## Evidence Measured in-process against a wiremock homeserver. Heap is glibc `mallinfo2` (in use + mmapped). A witness `Arc` captured by an event handler that `ClientInner` owns shows whether `ClientInner` was freed. | | before | after | |---|---|---| | client replaced 5x, each after a successful sync | 1 `ClientInner` never freed, 24-33 store fds stay open after evict | all freed, 0 fds | | 50 rooms, 1,500 events, then evict, +15 s, mock dropped | **8,510 KiB** still held | **794 KiB** (control without the event cache: 763 KiB) | So the harness leaks about 7.7 MiB per client at 50 rooms and 1,500 events. To make 250 MiB/day at about 24 replacements a day, each leaked client needs about 10 MiB. A larger account with an hour of traffic fits that figure. This is an inference: I did not measure a production client. A first version of the experiment had a mock that returned 404 for `/versions`. With that mock, no client ever reached `/sync` and every client was freed. The leak only shows up once the mock serves `/versions` and a key upload. AGENTS.md now records this. ## Fix Each cached client initialises `LatestEvents` itself, before its sync loop starts, on a one-worker tokio runtime that the entry owns (`LatestEventsRuntime`). The runtime is shut down (`shutdown_background`) when the entry is dropped. Tokio then drops the runtime's tasks, which breaks the cycle. `latest_event` behaviour is unchanged. The cost is one extra worker thread per live client. ## Other candidates, quantified (not the cause, not changed) - `rate_limit` per-bearer buckets (`bearer_read`, `bearer_write`, initialize `bearer`) are never pruned. That is about 250 B per entry per map, so about 750 B per distinct bearer. Even at one bearer every 5 min this is about 0.2 MiB/day. - `mas` introspection cache: soft cap of 256 with a sweep of expired entries, TTL ≤ 60 s. Bounded. - `last_used`: capped at 1024 entries. - `build_locks` and `retiring`: one small entry per `ClientKey`. A `Weak<Client>` pins only the 24-byte outer allocation, not `ClientInner`. - `session.rs` workers: removed on `close_session` and reaped when the worker transport drops. - `channel` PeerMap, PendingMap and ConfigCache: keyed per route, with TTL and cap where it matters. Bounded by the number of routes. ## Remaining growth to know about (not fixed here) While a client is alive, its event cache keeps every synced event in memory. LatestEvents listens to generic updates, not through a `RoomEventCacheSubscriber`, so auto-shrink never fires. The harness shows about 1.9 KB per 200-byte text event. A client rebuilt hourly stays bounded. A long-lived client, such as a `/channel` session with a non-expiring token, grows by that amount per event it receives. Bounding this means periodically subscribing to and dropping each room's event cache, which triggers the SDK's own shrink. That is a separate decision. ## SQLite `SQLITE_IOERR_GETTEMPPATH` (6410) The app cannot fix this cleanly in-process. `matrix-sdk-sqlite` opens its own connections and exposes no pragma hook, so there is no `temp_store = MEMORY`. Setting `SQLITE_TMPDIR` from Rust needs `std::env::set_var` (unsafe in edition 2024) or the `sqlite3_temp_directory` FFI, and the crate has `unsafe_code = "forbid"`. The deployment-side `emptyDir` on `/tmp` is the right fix. A no-code alternative is the env var `SQLITE_TMPDIR=<store dir>`, which puts temp files on the PVC. ## Second commit: RUSTSEC-2026-0433 The first CI run passed fmt, clippy and all 393 tests, then failed `cargo audit`. The cause was RUSTSEC-2026-0433 (`pulldown-cmark` 0.13.5, quadratic wikilink parsing, high), which reached the advisory database today. The lockfile had not changed, so `main` is affected too. `cargo update -p pulldown-cmark --precise 0.13.6` changes only the lockfile. The package is transitive via `ruma-events` markdown support and 0.13.6 declares rust-version 1.71.1. All gates were re-run after the bump. ## Verification - `cargo +1.98.0 fmt --all --check`: clean - `cargo +1.98.0 clippy --all-targets --all-features --locked -- -D warnings`: clean - `RUSTFLAGS=-Dwarnings cargo +1.98.0 test --all-features --locked`: 387 + 6 passed - `cargo +1.96.0 check --all-targets --all-features --locked`: ok (build floor) - crypto dependency floor unittest and check: ok - `cargo audit`: 0 vulnerabilities, with the two existing allowed unmaintained notices. `cargo deny check bans licenses sources`: ok - Mutation checks: removing the `LatestEventsRuntime::start` call, or keeping the runtime alive (`mem::forget`), each make `a_replaced_client_that_has_synced_is_freed` fail with "generation 0 was replaced but its client was never freed". Not merged, tagged or deployed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01E2xQRZDxzCiJjLb9uAhP9j

jlxq0 added 1 commit 2026-11-02 11:20:22 +00:00

fix(matrix-client): free replaced matrix-sdk clients ... b42e786b14

matrix-sdk 0.17 and 0.19.1 build LatestEvents with a strong
SendQueue::new(self.clone()) inside a task whose abort handle lives in
the same ClientInner, and the task only ends when one of three broadcast
channels owned by that client closes. With the event cache subscribed
(#64) the SDK initialises it in process_sync, so every client that
completed one sync was a reference cycle and was never freed after a
token change or an auth-expiry eviction replaced it: its stores and open
connections, its rooms and every event it had synced. Production replaces
a client each time its access token expires; RSS grew 200-300 MiB a day.

Each cached client now initialises LatestEvents itself, before its sync
loop starts, on a one-worker runtime it owns, and shuts that runtime down
when the entry is dropped. Tokio drops the runtime's tasks, which breaks
the cycle.

a_replaced_client_that_has_synced_is_freed replaces a client that has
completed a real sync three times, then evicts the last, and asserts each
ClientInner is freed through an event handler it owns. It fails with the
fix removed and with the runtime kept alive.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E2xQRZDxzCiJjLb9uAhP9j

jlxq0 added 1 commit 2026-11-02 11:28:03 +00:00

fix(deps): update pulldown-cmark to 0.13.6 for RUSTSEC-2026-0433 ... 36ab1c55bb

The advisory reached the RustSec database on 2026-11-02 and turned the
pull request's `cargo audit` step red with the lockfile unchanged.
pulldown-cmark is transitive through ruma-events' markdown support.
0.13.6 declares rust-version 1.71.1; check on 1.96.0, clippy and the full
suite on 1.98.0, audit and deny all pass.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E2xQRZDxzCiJjLb9uAhP9j

jlxq0 added 1 commit 2026-11-02 12:01:19 +00:00

release: v0.10.17 ... e8d4d7643c

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01E2xQRZDxzCiJjLb9uAhP9j

jlxq0 merged commit eccda5d988 into main 2026-11-02 12:04:14 +00:00

jlxq0 deleted branch fix/memory-leak-20261102 2026-11-02 12:04:14 +00:00

jlxq0 referenced this pull request from a commit 2026-11-02 12:04:15 +00:00

Merge pull request 'fix(matrix-client): free replaced matrix-sdk clients' (#158) from fix/memory-leak-20261102 into main

jlxq0 referenced this pull request from a commit 2026-11-02 12:31:58 +00:00

fix(matrix-mcp): v0.10.17, frees replaced matrix-sdk clients

jlxq0 referenced this pull request 2026-11-09 02:38:39 +00:00

fix(memory): cap SQLite pools and shrink the event cache periodically #159

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

blocked

Held on a named, checkable blocker: an issue number, a credential or a measurement. Not "held".

  

bug

Something isn't working

  

documentation

Improvements or additions to documentation

  

duplicate

This issue or pull request already exists

  

enhancement

New feature or request

  

good first issue

Good for newcomers

  

help wanted

Extra attention is needed

  

invalid

This doesn't seem right

  

question

Further information is requested

  

waiting-on-julian

Blocked on Julian. The ask is in the issue.

  

wontfix

This will not be worked on

No labels

blocked

bug

documentation

duplicate

enhancement

good first issue

help wanted

invalid

question

waiting-on-julian

wontfix

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

jlxq0/matrix-mcp!158

No description provided.