Skip to content

fix: recreate DTLS stack on ICE restart via opt-in config flag - #9

Merged
yakimenko73 merged 5 commits into
WebTrit:main_with_rtp_forwarders_extfrom
SERDUN:fix/wt-1167-recreate-dtls-on-ice-restart
Apr 23, 2026
Merged

fix: recreate DTLS stack on ICE restart via opt-in config flag#9
yakimenko73 merged 5 commits into
WebTrit:main_with_rtp_forwarders_extfrom
SERDUN:fix/wt-1167-recreate-dtls-on-ice-restart

Conversation

@SERDUN

@SERDUN SERDUN commented Apr 6, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Problem

When a peer creates a new `RTCPeerConnection` after an app restart (process kill), it sends a fresh DTLS `ClientHello` with no `renegotiation_info` extension. The existing connected `SSL*` object in Janus interprets this as an RFC 5746 renegotiation attempt and rejects it — because `renegotiated_connection` does not match the previous `verify_data`. DTLS stays stuck in `connecting` state, resulting in no audio after call restoration.

Additionally, `pc->connected` is never reset, so even with a new DTLS object the guard in `janus_ice_cb_nice_ready()` prevents the handshake from being re-entered after ICE reconnects.

Affects any scenario where the peer's process is killed mid-call and reconnects via ICE restart (re-INVITE / UpdateRequest).

Fix

Introduces an opt-in config flag `dtls_recreate_on_ice_restart` (default: `false`, vanilla Janus behavior unchanged).

When enabled, `janus_ice_restart()` schedules DTLS recreation as a GLib idle source on the handle's `mainctx` instead of doing it inline. This is the key design choice:

  • `janus_ice_cb_nice_recv` runs on the same GLib main loop
  • Scheduling via `g_idle_source_new()` + `g_source_attach(handle->mainctx)` serializes the destroy/create with any in-flight receive callbacks — no mutex on `pc->dtls`, no race
  • `handle->mutex` is held inside the callback when touching `handle->pc`, matching the pattern in `janus_ice_webrtc_free()`
  • `janus_refcount_increase/decrease` on the handle guards against the handle being freed before the idle fires (via `GDestroyNotify`)
  • `JANUS_ICE_HANDLE_WEBRTC_ICE_RESTART` is cleared in `janus_ice_restart()` immediately after `nice_agent_restart()` — not in the callback — since the flag means "restart was requested", not "DTLS is ready"

After the idle callback recreates the DTLS stack and resets `pc->connected = 0`, `janus_ice_cb_nice_ready()` re-enters the handshake path once ICE reconnects.

Changes

  • `src/ice.c` — `janus_ice_restart()`, `janus_ice_dtls_recreate_cb()`, global flag + accessors
  • `src/ice.h` — declarations for the two new accessors
  • `src/janus.c` — read `dtls_recreate_on_ice_restart` from `[nat]` config section
  • `conf/janus.jcfg.sample.in` — document the new option

Test

Tested on local WebTrit stack (Android device, app killed by OS mid-call):

[6086181492219027] ICE restart detected
[6086181492219027] Recreating DTLS stack after ICE restart
[6086181492219027]   Component is ready enough, starting DTLS handshake...
[6086181492219027] DTLS established, yay!
[6086181492219027] The DTLS handshake for the component 1 in stream 1 has been completed

Without this fix: DTLS stuck at `connecting`, zero audio after restoration.

Relation to upstream stance

Janus upstream recommends "create a new handle" for reconnection (issue meetecho#3021). This patch achieves the same effect — a fresh `SSL*` — within the existing handle so the SIP dialog with the proxy is preserved and call restoration is transparent to the user. The flag is disabled by default so existing deployments are unaffected.

Investigation

Full root cause analysis and test history: https://youtrack.portaone.com/issue/WT-1299

@SERDUN
fix: recreate DTLS SSL* and reset pc->connected on ICE restart (WT-1167)
13f716e 
When a peer creates a new RTCPeerConnection after an app restart (process
kill), it sends a fresh DTLS ClientHello with no renegotiation_info
extension. The existing connected SSL* object interprets this as an RFC
5746 renegotiation attempt and rejects it with a handshake_failure alert
because the renegotiated_connection field does not match the previous
verify_data. This leaves DTLS stuck in connecting state indefinitely and
results in no audio after call restoration.

Three changes to janus_ice_restart():

1. Destroy the DTLS retransmission timer (dtlsrt_source) before destroying
   the DTLS stack to avoid a dangling pointer in the timer callback.

2. Destroy the existing SSL* via janus_dtls_srtp_destroy() and create a
   fresh one via janus_dtls_srtp_create(). The new SSL* is in NEW state and
   handles the incoming ClientHello as a fresh handshake rather than a
   renegotiation.

3. Reset pc->connected to 0 so that janus_ice_cb_nice_ready() re-enters
   the DTLS handshake path when ICE reconnects. Without this reset the
   guard `if(pc->connected > 0) return` prevents the handshake from being
   enqueued after an ICE restart.

Tested on local WebTrit stack (Android, app killed by OS mid-call):
ICE restart → fresh DTLS handshake completes → audio restored.
@SERDUN
SERDUN marked this pull request as ready for review April 6, 2026 07:29
@SERDUN
SERDUN changed the base branch from master to main_with_rtp_forwarders_ext April 6, 2026 08:49
@yakimenko73
yakimenko73 self-requested a review April 6, 2026 13:38
yakimenko73
yakimenko73 requested changes Apr 6, 2026
View reviewed changes
Comment thread src/ice.c
@SERDUN
fix: add mutex, refcount safety and configurable dtls_recreate_on_ice…
807421f 
…_restart (WT-1299)

- Wrap DTLS destroy/create in handle->mutex inside janus_ice_restart(),
  matching the pattern used by janus_ice_webrtc_free(). Serializes against
  concurrent signaling-thread operations that also acquire handle->mutex.

- In janus_ice_cb_nice_recv(), take a short-lived refcount on pc->dtls
  before calling janus_dtls_srtp_incoming_msg() when the feature is enabled.
  Prevents use-after-free if janus_ice_restart() destroys pc->dtls on the
  signaling thread while an in-flight GLib callback still holds the pointer.

- Make the entire DTLS-recreate behaviour opt-in via dtls_recreate_on_ice_restart
  in the [nat] section of janus.jcfg. Default is false (vanilla Janus behaviour).
  Set to true to enable call restoration after peer process kill.

Addresses reviewer concern from WebTrit#9 (yakimenko73).
@SERDUN
SERDUN marked this pull request as draft April 14, 2026 09:40
SERDUN added 3 commits April 14, 2026 12:55
@SERDUN
fix: remove mutex from janus_ice_restart() to prevent deadlock (WT-1299)
c1b7b79 
janus_mutex is non-recursive (pthread_mutex_t). janus_ice_restart() is
called from janus.c:1685 with handle->mutex already held (locked at
1487), so adding a second lock on the same mutex causes a deadlock.
The earlier 807421f commit introduced this bug: nice_agent_restart()
ran but the DTLS recreate/reset block never executed.

Race with janus_ice_cb_nice_recv is mitigated by:
- refcount pattern in the callback (dtls_recreate_on_ice_restart guard)
- atomic destroyed flag in janus_dtls_srtp_destroy()
@SERDUN
fix: schedule DTLS recreate on GLib main loop to eliminate race (WT-1…
bf658b0 
…299)

Replace the refcount-based mitigation with a proper fix: schedule the
DTLS destroy/create inside an idle GSource attached to handle->mainctx.
Since janus_ice_cb_nice_recv also executes on the same GLib main loop,
the two operations cannot interleave — no mutex or refcount tricks needed.

The signaling thread calls nice_agent_restart() and then schedules
janus_ice_dtls_recreate_cb via g_idle_source_new(). The callback runs on
the handle's GLib main loop thread: it destroys the old DTLS context,
creates a fresh one, resets pc->connected, and clears ICE_RESTART flag.
A janus_refcount_increase/decrease on handle guards against handle
being freed before the idle fires.
@SERDUN
fix: address review blockers — mutex, ICE_RESTART flag, style (WT-1299)
657f672 
Blocker 2 (handle->pc race): acquire handle->mutex before reading handle->pc
in janus_ice_dtls_recreate_cb, matching the pattern in janus_ice_webrtc_free()
which sets handle->pc = NULL under the same lock. Release mutex before calling
janus_ice_webrtc_hangup() in the error path.

Blocker 3 (premature ICE_RESTART clear): move janus_flags_clear(ICE_RESTART)
from the idle callback into janus_ice_restart() immediately after
nice_agent_restart(). The flag means "restart was requested", not "DTLS ready".

Style: add blank line after static dtls_recreate_on_ice_restart; shorten the
config comment in janus.jcfg.sample.in to two lines; add note in callback
comment about janus_ice_cb_nice_ready triggering the new handshake.

Note: janus_refcount_increase after janus_dtls_srtp_create is intentional —
janus_ice_setup_local uses the same pattern (ice.c:3875). Without it,
janus_dtls_srtp_destroy() would bring refcount to 0 and free the object;
the subsequent janus_refcount_decrease in janus_ice_peerconnection_free
would then be a use-after-free.
@SERDUN SERDUN changed the title fix: recreate DTLS SSL* and reset pc->connected on ICE restart fix: recreate DTLS stack on ICE restart via opt-in config flag Apr 14, 2026
@SERDUN
SERDUN marked this pull request as ready for review April 14, 2026 12:11
@SERDUN
SERDUN requested a review from yakimenko73 April 14, 2026 12:11
@yakimenko73
yakimenko73 merged commit f289859 into WebTrit:main_with_rtp_forwarders_ext Apr 23, 2026
@SERDUN SERDUN mentioned this pull request Jun 1, 2026
Merged
3 tasks
SERDUN added a commit to SERDUN/janus-gateway that referenced this pull request Jun 1, 2026
@SERDUN
fix(ice): skip DTLS recreate on ICE restart when peer fingerprint unc…
d93ab51 
…hanged (WT-1540)

Follow-up to WebTrit#9 (which introduced dtls_recreate_on_ice_restart). When the
flag is on, janus_ice_restart() currently tears down the working DTLS
stack on every ICE restart, including plain network handovers where the
peer keeps the same PeerConnection and same DTLS certificate. The peer
never initiates a fresh handshake against the new server-side SSL*,
which times out after 20 seconds and drops the call.

Track whether the peer's a=fingerprint changed between the previous and
the current remote SDP, and only recreate the DTLS stack when it did.
Plain ICE restart (same cert) keeps the existing SSL*; restoration after
a force-stop / process kill (new cert) still triggers the original
recreate path so WT-1299 continues to work.

- ice.h: add JANUS_ICE_HANDLE_WEBRTC_DTLS_FINGERPRINT_CHANGED flag
- sdp.c: in janus_sdp_process_remote(), set/clear the flag in the ICE
  restart detection block by comparing the new rfingerprint with the
  cached pc->remote_fingerprint
- ice.c: guard the recreate path in janus_ice_restart() on the flag and
  log "DTLS fingerprint unchanged on ICE restart, keeping existing
  stack" when the recreate is skipped

Verified locally against WebTrit local stack: with the flag enabled and
this patch applied, Wi-Fi off/on during an active call now restores
audio cleanly with a single "fingerprint unchanged" log line instead of
the previous 20s DTLS timeout.
@SERDUN SERDUN mentioned this pull request Jun 2, 2026
Merged
3 tasks
yakimenko73 pushed a commit that referenced this pull request Jun 2, 2026
@SERDUN
fix(ice): skip DTLS recreate on ICE restart when peer fingerprint unc…
d9e62d9 
…hanged (WT-1540) (#12)

Follow-up to #9 (which introduced dtls_recreate_on_ice_restart). When the
flag is on, janus_ice_restart() currently tears down the working DTLS
stack on every ICE restart, including plain network handovers where the
peer keeps the same PeerConnection and same DTLS certificate. The peer
never initiates a fresh handshake against the new server-side SSL*,
which times out after 20 seconds and drops the call.

Track whether the peer's a=fingerprint changed between the previous and
the current remote SDP, and only recreate the DTLS stack when it did.
Plain ICE restart (same cert) keeps the existing SSL*; restoration after
a force-stop / process kill (new cert) still triggers the original
recreate path so WT-1299 continues to work.

- ice.h: add JANUS_ICE_HANDLE_WEBRTC_DTLS_FINGERPRINT_CHANGED flag
- sdp.c: in janus_sdp_process_remote(), set/clear the flag in the ICE
  restart detection block by comparing the new rfingerprint with the
  cached pc->remote_fingerprint
- ice.c: guard the recreate path in janus_ice_restart() on the flag and
  log "DTLS fingerprint unchanged on ICE restart, keeping existing
  stack" when the recreate is skipped

Verified locally against WebTrit local stack: with the flag enabled and
this patch applied, Wi-Fi off/on during an active call now restores
audio cleanly with a single "fingerprint unchanged" log line instead of
the previous 20s DTLS timeout.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@yakimenko73 yakimenko73 yakimenko73 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@SERDUN @yakimenko73