Skip to content

fix(client): release idle registry mutex allocations - #1884

Merged
jlucaso1 merged 1 commit into
mainfrom
fix/registry-mutex-retention-1883
Nov 10, 2026
Merged

jlucaso1 merged 1 commit into
mainfrom
fix/registry-mutex-retention-1883

Conversation

@jlucaso1

@jlucaso1 jlucaso1 commented Nov 10, 2026 •

Copy link
Copy Markdown
Member

Cold registry fallback initializes async-lock's event and leaves 72 extra bytes per client in the connected-idle retention check. This follows #1883, which was merged while the retention correction was being validated.

Registry operations now share an Arc-backed async-lock mutex through the complete holder/waiter lifetime. The final lease releases its strong reference and clears the weak slot under the admission lock, allowing both the mutex event and its allocation header to retire when idle. The actual guard drops before its lease. This adds temporary allocations at the start of an isolated operation; concurrent operations share them.

Four lifecycle regressions cover queued ordering, cancellation after notification, cancellation with another waiter, and simultaneous final lease retirement.

Validation:

  • All 100 ADV/recovery and mutex lifecycle cases passed on the pre-rebase tree; this file is byte-for-byte identical after applying the patch to main fa8c640c.
  • Current-main all-feature library Clippy passes with warnings denied; the no-default-feature wasm32 library check passes. Formatting and diff checks pass.
  • Exact-head connected retention, CPU/allocation comparisons and SDK CI are queued. Full SDK tests on the updated base have not completed.

The existing Cargo Deny failure for RUSTSEC-2026-0459 is inherited from main's bundled SQLite dependency. Published Diesel 2.3.14 excludes the fixed libsqlite3-sys 0.39 line; the audit remains enabled.

@coderabbitai

coderabbitai Bot commented Nov 10, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

@greptile-apps

greptile-apps Bot commented Nov 10, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 5/5

[Medium impact] The PR appears safe to merge.

Summary

Releases the registry mutex allocation when the last holder or waiting operation finishes.

  • Registry operations share a mutex that disappears when the client is idle.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Registry operation starts] --> B[Lock weak slot]
  B --> C[Reuse or create shared mutex]
  C --> D[Keep lease and release slot lock]
  D --> E[Wait for registry mutex]
  E --> F[Hold registry guard]
  F --> G[Release guard before lease]
  E --> H[Cancellation drops acquisition before lease]
  G --> I[Drop lease under slot lock]
  H --> I
  I --> J{Strong references remain?}
  J -->|Yes| K[Keep weak slot]
  J -->|No| L[Clear weak slot]
Loading

Reviews (1) · Last reviewed commit: "fix(client): retire idle registry mutex ..." · Reviewed by Greptile

@jlucaso1

Copy link
Copy Markdown
Member Author

@codex review

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 10, 2026 •

Copy link
Copy Markdown
Codex Review Summary: ✅ Completed

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-11-10T22:50:47.066178Z e3a67b9 Draft marked ready
🔒 Security Review ✅ Completed 2026-11-10T22:54:29.613660Z e3a67b9 Draft marked ready
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. What shall we delve into next?

Reviewed commit: e3a67b9c37

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@jlucaso1
jlucaso1 marked this pull request as ready for review November 10, 2026 22:49
@jlucaso1
jlucaso1 merged commit 77910a9 into main Nov 10, 2026
31 of 34 checks passed
@jlucaso1
jlucaso1 deleted the fix/registry-mutex-retention-1883 branch November 10, 2026 22:49
@codspeed

codspeed Bot commented Nov 11, 2026 •

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

1 warning, 1 note

⚠️ 141 benchmarks are dominated by system calls

System calls cannot be simulated consistently and are left out of the measure. The reported value understates the real cost. Switch to the Walltime instrument to measure them.

Learn more about system calls

See affected benchmarks

ℹ️ 24 benchmarks have no flamegraph

The flamegraph wasn't generated: most of the time was spent in runtime internals, which would leave nothing useful to analyze.

See affected benchmarks

✅ 852 untouched benchmarks
🆕 472 new benchmarks
⏩ 372 skipped benchmarks1

Performance Changes

Mode Benchmark BASE HEAD Efficiency
🆕 Memory protocol_address_per_device[128_round0] N/A 0 B N/A
🆕 Memory protocol_address_per_device[128_round1] N/A 0 B N/A
🆕 Memory protocol_address_per_device[128_round2] N/A 0 B N/A
🆕 Memory protocol_address_per_device[128_round3] N/A 0 B N/A
🆕 Memory protocol_address_per_device[32_round0] N/A 0 B N/A
🆕 Memory protocol_address_per_device[32_round1] N/A 0 B N/A
🆕 Memory protocol_address_per_device[32_round2] N/A 0 B N/A
🆕 Memory protocol_address_per_device[32_round3] N/A 0 B N/A
🆕 Memory protocol_address_per_device[512_round0] N/A 0 B N/A
🆕 Memory protocol_address_per_device[512_round1] N/A 0 B N/A
🆕 Memory protocol_address_per_device[512_round2] N/A 0 B N/A
🆕 Memory protocol_address_per_device[512_round3] N/A 0 B N/A
🆕 Memory protocol_address_per_device[128_round0] N/A 0 B N/A
🆕 Memory protocol_address_per_device[128_round1] N/A 0 B N/A
🆕 Memory protocol_address_per_device[128_round2] N/A 0 B N/A
🆕 Memory protocol_address_per_device[128_round3] N/A 0 B N/A
🆕 Memory protocol_address_per_device[32_round0] N/A 0 B N/A
🆕 Memory protocol_address_per_device[32_round1] N/A 0 B N/A
🆕 Memory protocol_address_per_device[32_round2] N/A 0 B N/A
🆕 Memory protocol_address_per_device[32_round3] N/A 0 B N/A
... ... ... ... ... ...

ℹ️ Only the first 20 benchmarks are displayed. Go to the app to view all benchmarks.


Comparing fix/registry-mutex-retention-1883 (e3a67b9) with main (92e64c4)2

Open in CodSpeed

Footnotes

  1. 372 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

  2. No successful run was found on main (fa8c640) during the generation of this report, so 92e64c4 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant