Commit b781447
Disable WebRTC-IceHandshakeDtls field trial (livekit#1015)
Resolves livekit#929.
## Summary
Disable the `WebRTC-IceHandshakeDtls/Enabled/` field trial that PR livekit#911
turned on by default in 2.13. The DTLS-in-STUN piggyback feature is the
only SDK-side delta between 2.10.2 (which did not exhibit this crash)
and 2.14.0 (which does) that touches the ICE / DTLS / Port surface.
## Why
The crash signature reported in livekit#929 (re-opened after livekit#933):
- Symbolicated against the exact `144.7559.04` framework binary, the
crash PC is a 2-instruction setter (`str xzr, [x0, #0x28]; ret`) — a
wild-pointer store, not a hardening abort.
- Frames 6–10 sit in the `p2p_transport_channel.cc` compilation unit and
exhibit the canonical `scoped_refptr::Release()` pattern (`ldaxr/stlxr`
retry loop) immediately after a virtual call.
- Frames 4–5 sit in `port.cc` and the surrounding function contains
literal-pool references to libc++ hardening strings for `destroy_at` and
`vector::erase` — i.e. the function is iterating a `std::vector` of
Port/Connection objects.
- Bottom frames are `Thread::PreRun → ProcessMessages →
Dispatch(AnyInvocable)` — a posted task on the WebRTC worker thread.
The call shape matches the destructor cascade:
```
Thread::Dispatch
→ ~DtlsTransportInternalImpl
→ ice_transport()->ResetDtlsStunPiggybackCallbacks() // p2p/dtls/dtls_transport.cc:269
→ P2PTransportChannel::ResetDtlsStunPiggybackCallbacks()
→ for (connection : connections_)
connection->DeregisterDtlsPiggyback() // p2p/base/p2p_transport_channel.cc:2387
```
That entire callback-deregister iteration only exists when the field
trial is active — `dtls_in_stun_` in `DtlsTransportInternalImpl` is
initialized from `field_trials().IsEnabled("WebRTC-IceHandshakeDtls")`
and otherwise no piggyback callbacks are ever installed on `Connection`.
## Why not fix upstream
I reviewed every commit on `upstream/branch-heads/{7559, 7694, 7695,
7696, 7850, 7851}` and `upstream/master` (tip `07ea9cc4a1`, 2026-05-21)
touching `p2p/base/{connection,port,p2p_transport_channel}`,
`p2p/dtls/{dtls_transport,dtls_stun_piggyback_controller}`, and
`rtc_base/callback_list`, plus all 70 CLs under `webrtc:367395350`.
- `~P2PTransportChannel`, `~Connection`, `~Port`,
`ResetDtlsStunPiggybackCallbacks`, `DeregisterDtlsPiggyback`,
`RemoveConnection`, `Connection::Destroy` — **byte-for-byte identical**
master vs. `branch-heads/7559`.
- `connections_` type — still `std::vector<Connection*>`,
network-thread-guarded, on every branch.
- Only material upstream deltas are defensive: `a4dd38a556` (m147+)
null-checks `ice_transport()` before the call, and `19e62b7f55` (m152+)
gates the destructor on `if (dtls_in_stun_)`. Neither fixes the
iteration body for users with the trial enabled — they only protect the
trial-off case, which this PR achieves in one line.
- No commit explicitly labels itself as a crash / race / UAF fix in this
surface on any branch through master.
Upstream still ships this regression on tip. The trial is intentionally
a kill-switch (see `g3doc/field-trials.md` — "Not enabled by default;
experimentation required") and the upstream registry has its expiry date
(`2026-01-01`) already passed without promotion.
## Cross-platform check
`gh search code "WebRTC-IceHandshakeDtls" --owner livekit` returns
exactly one hit org-wide — this file. Android, JS, Flutter, React
Native, and Rust SDKs all leave the trial at upstream's default-off,
which is consistent with the crash being iOS-only.
## Tradeoff
Removes the ~1–2 RTT call-setup saving that the piggyback feature
provides on supporting servers. Falls back to the same DTLS-after-ICE
sequencing as ≤2.12. Worth giving back to stop the crash; we can
re-evaluate when upstream actually fixes the destructor surface (likely
behind an opt-in `RoomOptions` flag in a later release).
## Hypothetical symbolicated stack
The Crashlytics report only shows nearest-export-symbol names, but
mapped against `144.7559.04`'s binary the addresses resolve to this
approximate call chain:
```
0 <inline destructor helper> (str xzr, [x0, #0x28]; ret)
1 <vector teardown> in frequency_tracker.o ← stripped internal
2 <vector teardown> in frequency_tracker.o ← stripped internal
3 <vector teardown> in frequency_tracker.o ← stripped internal
4 <port.cc internal> near vector::erase / destroy_at hardening literals
5 <port.cc internal> ldr…blr — virtual call into Connection vtable+0xc0
6 P2PTransportChannel::ResetDtlsStunPiggybackCallbacks ← iterates connections_
7 P2PTransportChannel internal helper
8 P2PTransportChannel internal helper (RTC_LOG severity 'E')
9 P2PTransportChannel internal helper
10 ~DtlsTransportInternalImpl ← scoped_refptr Release pattern
11 webrtc::Thread::Dispatch(absl::AnyInvocable<void()&&>)
12 webrtc::Thread::ProcessMessages
13 webrtc::Thread::PreRun
```
## Test plan
- [ ] CI green on `macOS / iOS / Catalyst / visionOS / tvOS` matrix
- [ ] TestFlight build with this change validated by users on livekit#929
(`@SlavianaKaziuko`, `@matthewcheok`, `@sergeyphi`)
- [ ] Confirm `LKRTCPeerConnectionFactory` still configures without that
line (no crash on factory init)
- [ ] Spot-check a session connect + disconnect cycle locally to confirm
DTLS still negotiates (just no longer piggybacked inside STUN)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
(cherry picked from commit 99490ff)1 parent 077a607 commit b781447
2 files changed
Lines changed: 1 addition & 3 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
96 | 96 | | |
97 | 97 | | |
98 | 98 | | |
99 | | - | |
100 | | - | |
101 | | - | |
102 | 99 | | |
103 | 100 | | |
104 | 101 | | |
| |||
0 commit comments