Skip to content

fix(android): prevent TLS key exhaustion at UI startup - #10244

Closed
proggeramlug wants to merge 2 commits into
mainfrom
codex/10219-android-tls-pool
Closed

proggeramlug wants to merge 2 commits into
mainfrom
codex/10219-android-tls-pool

Conversation

@proggeramlug

@proggeramlug proggeramlug commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Android UI startup touches enough Rust OS-backed thread locals to exhaust bionic’s process-wide pool of 128 pthread keys. On base d8bfa28a38, the issue’s minimal app aborts immediately after start_timer_pump, with the backtrace in Rust’s LazyKey::lazy_init while resolving PROCESSING_REJECTIONS.

Use one pthread key for Android perry_thread_local! storage and the hot-address cache. Values remain separately typed and lazily initialized per thread, with stable System-allocated addresses and indexed lookup. Cleanup preserves reverse initialization order, permits access to live values and initialization of new values from destructors, retires cached addresses before dropping values, and retains a tombstone across later pthread destructor passes. Other targets keep std’s storage and exact error type; Android try_with uses an equivalent local AccessError because std’s non-exhaustive error cannot be constructed outside std.

Validation:

  • Base app: native Android SIGABRT immediately after the timer pump starts. Final fixed app: compiled and packaged with matching compiler/runtime identities, survived three independent launches, displayed hello in UIAutomator, and stayed running without a TLS abort.
  • Host runtime: 3,790 unit tests passed, four ignored; new integration test passed.
  • Android runtime: all 21 focused TLS tests passed, including provider sharing, initial cache fill, and teardown of cached values.
  • Android integration: 192 actual Perry declarations on eight concurrent workers preserve isolated values and perform 1,536 worker drops exactly once; access from a later external pthread destructor returns AccessError. Both tests passed.
  • Five storage-backend tests passed natively on Android and with host AddressSanitizer: table growth while borrowing a value, 512 declarations, thread turnover, aligned allocations, initializer panic/reentrancy, and destructor access/order.
  • The standalone TLS-policy audit still reports the base raw declaration in regex/perex_owner.rs; a baseline copy produces identical diagnostics.
  • The new integration suite is selected automatically by ci_e2e_scope.py for this PR.
  • Formatting, Node-version consistency, GC root-holder audit, GC store-site inventory, and the other quick pre-tag checks pass. Public benchmark freshness remains a base failure; all 69 checked artifact, verifier, gate, and fingerprint paths are byte-identical to the base.

Android validation uses an isolated Android 15/API 35 arm64 emulator and an API 24 NDK linker. The report used Android 14/API 34 x86_64 with arm64 translation; that translated configuration and physical hardware were not tested. No version bump.

Closes #10219.

Summary by CodeRabbit

  • Bug Fixes

    • Improved Android runtime support for applications using many thread-local values, avoiding failures caused by platform thread-local storage limits.
    • Preserved per-thread initialization and cleanup behavior, including reliable cleanup when worker threads exit.
    • Accessing thread-local values after runtime teardown now reports an error instead of returning invalid data or triggering unexpected behavior.
  • Tests

    • Added coverage for high-volume thread-local usage, cleanup ordering, initialization failures, recursive initialization, and worker-thread turnover.

@coderabbitai

coderabbitai Bot commented Sep 14, 2026

Copy link
Copy Markdown

Review Change StackReview Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: ba6a2484-bf84-4303-8f1b-953147fb1e2a

📥 Commits

Reviewing files that changed from the base of the PR and between 9fda98d and 24a5726.

📒 Files selected for processing (7)
  • changelog.d/10244-android-tls-pool.md
  • crates/perry-runtime/src/lib.rs
  • crates/perry-runtime/src/tls_hot.rs
  • crates/perry-runtime/src/tls_hot/provider_tests.rs
  • crates/perry-runtime/src/tls_os_pool.rs
  • crates/perry-runtime/src/tls_os_pool/tests.rs
  • crates/perry-runtime/tests/android_tls_pool.rs

Included review availability: Your plan provides up to 8 included reviews per hour; 5 remain after this review.


📝 Walkthrough

Walkthrough

Changes

Android pooled TLS storage

Layer / File(s) Summary
Pooled TLS storage
crates/perry-runtime/src/tls_os_pool.rs
Adds shared pthread-key storage with indexed per-thread values, lazy initialization, ordered destruction, recursive-initialization checks, and fallible access after teardown.
Cached TLS integration
crates/perry-runtime/src/lib.rs, crates/perry-runtime/src/tls_hot.rs, crates/perry-runtime/src/tls_hot/provider_tests.rs, changelog.d/10244-android-tls-pool.md
Uses the pooled backend for Android cached TLS declarations and retains standard TLS storage on other targets. Updates AccessError handling and documents the change.
TLS lifecycle validation
crates/perry-runtime/src/tls_os_pool/tests.rs, crates/perry-runtime/tests/android_tls_pool.rs
Tests initialization recovery, destruction order, alignment, per-thread isolation, high declaration counts, and protection against value resurrection after teardown.

Priority: ⬆️ High

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Bug fix · Severity of issue fixed: Medium

Sequence Diagram(s)

sequenceDiagram
  participant HotKey
  participant tls_hot
  participant tls_os_pool
  participant pthread_key
  HotKey->>tls_hot: try_with
  tls_hot->>tls_os_pool: resolve pooled LocalKey
  tls_os_pool->>pthread_key: access shared per-thread pool
  pthread_key-->>tls_os_pool: pool or destroyed state
  tls_os_pool-->>tls_hot: value or AccessError
  tls_hot-->>HotKey: Result
Loading

Merge Risk: ⚪ Minimal · up to 24a57

The Android TLS pooling change has no identified merge-blocking issue in the supplied review evidence.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 41.38% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 29 functions across 6 files. (1 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the Android TLS key exhaustion fix that is the main change in the pull request.
Description check ✅ Passed The description provides a clear summary, detailed changes, related issue, extensive test results, platform details, and checklist-relevant information. It does not reproduce the template headings or …
Linked Issues check ✅ Passed The PR addresses #10219. On Android, tls_os_pool::LocalKey stores all declared values behind one shared pthread_key_t, while each declaration keeps typed, lazy initialization and indexed lookup. `…
Out of Scope Changes check ✅ Passed The changes stay within #10219. The pool implementation, tls_hot integration, backend tests, Android integration tests, and changelog all support the Android pthread-key exhaustion fix or document i…
Full details: Docstring Coverage

Explanation

Docstring coverage is 41.38% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 29 functions across 6 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/10219-android-tls-pool

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

Comment @coderabbitai help to get the list of available commands.

@proggeramlug

Copy link
Copy Markdown
Contributor Author

Landed via merge train #10256 (v0.5.1565): #10256. The train includes the audited PR changes and its integration fixes; merged main matches the validated train tree.

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.

android: a minimal perry/ui app aborts at startup with "fatal runtime error: out of TLS keys"

1 participant