Repository navigation
fix(android): prevent TLS key exhaustion at UI startup - #10244
proggeramlug wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (7)
Included review availability: Your plan provides up to 8 included reviews per hour; 5 remain after this review. 📝 WalkthroughWalkthroughChangesAndroid pooled TLS storage
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
Merge Risk: ⚪ Minimal · up to 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)
✅ Passed checks (4 passed)
Full details: Docstring CoverageExplanation 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.)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
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. Comment |
…rtual pthread keys Two Android defects stood between a linked Perry v0.5.1520 library and a green run. Bionic hands out malloc pointers tagged 0xb4 in the top byte, and Perry range-checks addresses as plain 48-bit values. Every stdlib-linked program (all of the three-import cases) then read its arena objects as shapeless: stores past the second slot were dropped and console.log printed empty strings. tn_so_runner turns heap tagging off before loading; an app does the same with allowNativeHeapPointerTagging="false". Found with lldb on the Pixel: the first object after js_object_alloc_with_shape arrived as 0xb400007d63364a30. Perry's runtime declares several hundred thread_local!, and Android's std keeps each in its own pthread key, so a library aborted with "out of TLS keys" at bionic's 128. tn_pthread_keys.c virtualizes the four key calls and reaches the link line through PERRY_EXTRA_LINK_ARGS, so neither the compiler nor the runtime archive changes. Upstream pools its own declarations in PerryTS/perry#10244, in no release yet. unsupported-export imported Raycaster, which PRD-531 implemented; it now imports CatmullRomCurve3, which the catalog still refuses.
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 afterstart_timer_pump, with the backtrace in Rust’sLazyKey::lazy_initwhile resolvingPROCESSING_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; Androidtry_withuses an equivalent localAccessErrorbecause std’s non-exhaustive error cannot be constructed outside std.Validation:
helloin UIAutomator, and stayed running without a TLS abort.AccessError. Both tests passed.regex/perex_owner.rs; a baseline copy produces identical diagnostics.ci_e2e_scope.pyfor this PR.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
Tests