Latest FFI with memory leak fixes - #829
Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
Devin Review found 2 potential issues.
1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
| @@ -1 +1 @@ | |||
| Subproject commit bae4df2f3d6c8f0f6ce81a7370c445779f1948d2 | |||
| Subproject commit 5a656c4bc4bc5c4f7fad17ab69044db3b487dfeb | |||
There was a problem hiding this comment.
🔴 Signal-only resumes always time out
With no PC transition, wait_pc_connection_inner waits for a notification instead of its three-second deadline. The outer timeout forces a full reconnect, interrupting healthy media.
Learn more
The resume logic treats an unchanged Connected state as healthy after settle_delay. A signal-only outage normally produces no peer-connection transition, so pc_state_notify has nothing to wake the loop. The deadline only affects a boolean checked before awaiting that notification. tokio::time::timeout(ICE_CONNECT_TIMEOUT, wait_connected) eventually fails the resume after 15 seconds, and the engine escalates to a full reconnect.
Example: A websocket briefly disconnects while both media transports remain connected. The first recovery check runs before three seconds and waits on pc_state_notify. No connection state changes, so the check never runs again at three seconds; the resume fails at 15 seconds instead of succeeding.
Recommended fix: Include a sleep until the settle deadline in the loop's wait condition, alongside pc_state_notify.notified(). Re-evaluate all transports when either completes, while preserving the outer ICE_CONNECT_TIMEOUT.
Was this helpful? React with 👍 or 👎 to provide feedback.
| @@ -1 +1 @@ | |||
| Subproject commit bae4df2f3d6c8f0f6ce81a7370c445779f1948d2 | |||
| Subproject commit 5a656c4bc4bc5c4f7fad17ab69044db3b487dfeb | |||
There was a problem hiding this comment.
🔴 Dead peer connections pass resume validation
After three seconds, transport_recovered accepts an unchanged Connected state. Libwebrtc retains that stale state for tens of seconds, so a later notification can validate dead media.
Learn more
A remote peer can disappear while libwebrtc continues reporting Connected until its receiving or consent timeout. In that interval, neither generation counter changes, which is indistinguishable from a healthy signal-only outage. The new logic resolves that ambiguity after PC_RECONNECT_SETTLE_DELAY, but the delay is only three seconds while the same code documents stale state lasting tens of seconds. Any notification that causes reevaluation after three seconds can therefore accept the stale transport and report a successful resume.
Example: The server node hosting the subscriber transport disappears, while signaling resumes on another node. The old subscriber connection still reports Connected; a publisher transition wakes the shared notifier after three seconds. transport_recovered accepts the subscriber, and the room reports reconnection without receiving media.
Recommended fix: Do not accept an unchanged pre-resume state before libwebrtc's stale-connection window expires. Use explicit transport recovery evidence where available, or set and verify a deadline that exceeds the configured ICE receiving timeout. Add an integration test where the remote transport disappears but remains locally Connected beyond three seconds.
Was this helpful? React with 👍 or 👎 to provide feedback.
3 participants
Pulls in https://github.com/livekit/rust-sdks/releases/tag/livekit-ffi%2Fv0.12.80, mainly resolving memory leak reported by SDK customer.