- prevent some escape to heap
- avoid copying by using a ring buffer for receiver reports (probably
should remove this as this is for debugging only and data so far has
shown clients sending bad data and nothing more.)
* Close peer connection unconditionally to unblock set local/remote
description operations.
Have been chasing a leak where participants have a lot of connectivity
issues and analysed a goref with Claude. Output below.
Jo Turk quickly patched sctp for reported issue -
https://github.com/pion/sctp/pull/465.
This PR moves the peer connection close to before waiting for events
queue to be drained as event queue could be blocked on
`SetLocal/RemoteDescription` hanging.
The scenario is a bit far-fetched as a lot of things have to happen, but
it does point to a scenario where things could hang. Remains to be seen
if this helps. Note that closing the peer connection early could mean
the contained objects (like data channels) could all be closed as part
of the peer connection close. But, still keeping the explicit clean up
path (which should effectively become no-op) to minimise changes.
------------------------------------------------------------------
The wedge is in pion/sctp's blocking-write gate, called synchronously from inside the PC's operations queue. Five things have to be true at the same time, and on this build they all are:
1. SCTPTransport.Start is synchronous in the SetRemoteDescription op
The stuck stack:
PeerConnection.SetRemoteDescription.func2 (peerconnection.go:1363)
→ startRTP → startSCTP
→ SCTPTransport.Start (sctptransport.go:141)
→ DataChannel.open (datachannel.go:178)
→ datachannel.Dial → Client → Stream.WriteSCTP
→ Association.sendPayloadData (association.go:3141) ← blocks here
SCTPTransport.Start synchronously sends the DCEP "OPEN" for each pre-negotiated channel. The operations.start goroutine runs SetRemoteDescription's logic; it does not return until Start does.
2. The wait has no deadline
Stream.WriteSCTP (stream.go:289) calls sendPayloadData(s.writeDeadline, ...). s.writeDeadline is the default zero-value deadline.Deadline — never armed, because DataChannel.Dial doesn't call Stream.SetWriteDeadline. So the <-ctx.Done() arm of the wait select can
never fire.
3. EnableDataChannelBlockWrite(true) puts SCTP into a serialized-write gate
At livekit-server/pkg/rtc/transport.go:362 livekit calls se.EnableDataChannelBlockWrite(true). That flips the sendPayloadData path to:
// association.go:3138-3148
if a.blockWrite {
for a.writePending {
a.lock.Unlock()
select {
case <-ctx.Done(): // never (no deadline)
case <-a.writeNotify: // only fires when writeLoop fully drains pendingQueue
}
a.lock.Lock()
}
a.writePending = true
}
4. writeNotify only fires after the writeLoop drains everything
The only place notifyBlockWritable is called is gatherOutbound (association.go:3085-3088), and only when len(chunks) > 0 && a.pendingQueue.size() == 0 — i.e., the writeLoop actually managed to move all pending chunks to inflight. If cwnd is full and SACKs stop
arriving, the writeLoop wakes up, sees zero room, sends nothing, and writePending stays true.
5. There is no association-level abort timer for data writes
At association.go:764:
assoc.t3RTX = newRTXTimer(timerT3RTX, assoc, noMaxRetrans, rtoMax)
noMaxRetrans means the retransmission timer never gives up. INIT has maxInitRetrans, but data does not. There is no equivalent of TCP's tcp_retries2 → ETIMEDOUT → ABORT. So once the path is dead post-handshake, t3RTX keeps firing into the void and the association
never transitions out of established on its own.
What it takes to wake it up
Only an external close: somebody has to terminate the underlying DTLS conn (which makes Association.readLoop's netConn.Read fail, which closes closeWriteLoopCh, which lets timerLoop exit). But — and this is the kicker — readLoop's defer at association.go:976-996
closes everything except it does not call notifyBlockWritable. So even if readLoop unwinds, any goroutine parked on <-a.writeNotify stays parked unless it was watching ctx (which here it isn't).
So the trigger sequence on this pod was almost certainly:
1. Peer establishes ICE+DTLS+SCTP, association goes established.
2. Peer disappears (ICE silently fails, NAT rebinding, OS sleep, kill -9, etc.).
3. The first DCEP-OPEN for one of livekit's pre-negotiated channels is queued; cwnd never opens because no SACKs return.
4. writePending is now true for the lifetime of the process, with no deadline, no ctx, no kill.
5. The PC's operations queue is wedged, SetRemoteDescription never returns, livekit-server's handleRemoteOfferReceived event handler is parked, the participant is never torn down, and the SCTP timerLoop pins the entire participant graph in memory until OOM-kill.
Realistic fixes (in order of how clean they are)
1. Upstream: in pion/sctp, broadcast notifyBlockWritable() (or close writeNotify) inside readLoop's defer cleanup, so a closed association unblocks any pending writers. This is the right fix.
2. livekit-server: wrap pc.SetRemoteDescription(...) with a timeout, and on timeout call pc.Close() — Close ultimately tears down the DTLS conn, which lets readLoop exit (point 1 still needs to be true for the writer goroutine to actually unblock, though).
3. Workaround: call stream.SetWriteDeadline(...) on the SCTP stream before issuing the DCEP open, so the ctx arm of the select can fire. Requires reaching past webrtc.DataChannel though.
4. Heaviest hammer: don't pre-negotiate the data channels inline with SetRemoteDescription — open them lazily after PC reaches connected so a stuck open never blocks signaling.
Without (1), even (2) leaves the writer goroutine itself parked forever — but at least the PC and its participant-side state would be released; only the SCTP goroutine subtree (much smaller) would leak.
* revert probe stop change
* handle nil offer
`iceServersForParticipant` builds UDP TURN URLs by interpolating the
node IP directly into a format string:
fmt.Sprintf("turn:%s:%d?transport=udp", ip, port)
When `NodeIP.V6` is set, `ToStringSlice()` includes the bare IPv6
address, producing URLs like:
turn:2a05:d014:ee4:1201:7039:38c:f652:a252:443?transport=udp
RFC 3986 §3.2.2 requires IPv6 addresses in URIs to be enclosed in
square brackets. Without them the port is ambiguous and WebRTC clients
(e.g. libdatachannel) reject the URL with "Invalid ICE server port".
Use `net.JoinHostPort` which handles bracketing for IPv6 and is a
no-op for IPv4, producing well-formed URLs:
turn:[2a05:d014:ee4:1201:7039:38c:f652:a252]:443?transport=udp
turn:1.2.3.4:443?transport=udp
* Use Muted in TrackInfo to propagated published track muted.
When the track is muted as a receiver is created, the receiver
potentially was not getting the muted property. That would result in
quality scorer expecting packets.
Use TrackInfo consistently for mute and apply the mute on start up of a
receiver.
* update mute of subscriptions
* fix: ensure num_participants is accurate in webhook events (#4265)
Three fixes for stale/incorrect num_participants in webhook payloads:
1. Move participant map insertion before MarkDirty in join path so
updateProto() counts the new participant.
2. Use fresh room.ToProto() for participant_joined webhook instead of
a stale snapshot captured at session start.
3. Remove direct NumParticipants-- in leave path (inconsistent with
updateProto's IsDependent check), force immediate proto update,
and wait for completion before triggering onClose callbacks.
* fix: use ToProtoConsistent for webhook events instead of forcing immediate updates
* Update go deps
Generated by renovateBot
* update api usage
---------
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: David Zhao <dz@livekit.io>
* Log join duration.
Also revert the "unresolved" init. Defeated the purpose of log resolver
as it was resolving with those values even if not forced. Instead set it
to "unresolved" if not set when forced.
Join duration is not reset if resolver is reset as that happens on
moving a participant and there is no new join duration in that case.
* explode
Some e2e is failing due to subscriptions happening late and the expected
order of m-lines is different. Not a hard failure, but logging more to
make seeing this easie.
For a participant migrating out, the track could be resumed on a
different node, but ending on the migrating out node. So, `flush` should
be used to indicate if track is going to be resumed.
Subscription can switch between remote track and local track or
vice-versa. When that happens, closing the subscribed track of one or
the other asynchronously means the re-subscribe could race with
subscribed track closing.
Keeping the case of `isExpectedToResume` sync to prevent the race.
Would be good to support multiple subscribed tracks per subscription.
So, when subscribed track closes, subscription manager can check and
close the correct subscribed track. But, it gets complex to clearly
determine if a subccription is pending or not and other events. So,
keeping it sync.
Removing some logs which have not been useful in terms of insights other
than saying that there are a bunch of packets missing. Going to start
looking at gaps in terms of time if the inter-packet gap is too high.
Also, using logging these events as first 20 and then every 200.
A bunch of edges to note here
RED packet does not have sequence number for redundant blocks. It only
has timestamp offset compared to the primary payload. The receivers are
supposed to use just timestamp to sequence the payload and decode.
But, when converting from RED -> Opus, the packets extracted from RED
packet should be assigned a sequence number before they can be
forwarded. The simple rule is, if packet N contains X redundant
payloads, they are assigned sequence number of N - X to N - 1.
However there are cases like the following sequence (with 1 packet
redundancy)
- Seq num 10, timestamp 2000, forwarded
- Seq num 11 is lost
- Seq num 12 has a redundant payload. Seq num 12 has timestamp of 4000.
Ideally would expect the redundant payload to have a timestamp offset
of 1000, so the redundant payload can be mapped to sequence number 11
and timestamp 3000 (4000 - 1000). But, in the problematic case, it has
an offset of 3000 resulting in sequence number 11 and timestamp of
1000 causing an inversion with packet at sequence number 10.
Unclear if this a publisher issue, i. e. packing RED wrong or if this is
some expected behaviour with DTX. i. e. the DTX packets are not included
in redundant payload. For example, the sequence
- Seq num 10 -> DTX
- Seq num 11 -> DTX -> lost
- Seq num 12 -> Regular packet and include sequence num 9 as that is the
last regular packet.
Anyhow, detect this condition and drop the time inverted packet.
Note however this handles only inversion against the highest sent packet
sequence number and timestamp. So, some old packet inverted with some
other old packet getting forwarded will get through. That has been the
case always though and detecting that would be expensive and
complicated.
At least for egress, will also look at adding a check for inversion so
that it can catch it before sending it down the gstreamer pipeline. As
the egress uses a jitter buffer with ordered sequence number emits, it
will be simpler to detect timestamp going back when sequence number is
moving forward (of course the mute/dtx challenege is there).
* Log time inversion between incoming packets
Log of timestamp inversion within a red packet did not show anything.
Log across packets. Not dropping till there is more evidence of the
cause.
* save
* comment
* Guard against timestamp inversion in RED -> Opus conversion.
Seeing timestamp inversion (sequence number is +1, but timestamp is
-960, i.e. 20ms) in the RED -> Opus conversion path. Not able to spot
any bugs in code. So, logging details upon detection and also dropping
the packet. If not dropped, downstream components like Egress treat it
as a big timestamp jump (because sequence number is moving forward) and
try to adjust pts which ends up causing drops.
* do not log time reversal at the start
* typo
When a subscriber disconnects, observer closures registered on the
publisher's TrackChangedNotifier and TrackRemovedNotifier were never
removed. These closures capture the SubscriptionManager, which holds
the ParticipantImpl, preventing the entire participant object graph
(PCTransport, SDPs, RTP stats, DownTracks) from being garbage collected.
In rooms with many participants that disconnect and reconnect frequently,
this causes unbounded memory growth proportional to the number of
disconnect events. The leaked memory is not recoverable while the room
remains open.
Clear notifiers in both handleSubscribedTrackClose (individual
subscription teardown) and SubscriptionManager.Close (full participant
teardown), matching the existing cleanup in handleSourceTrackRemoved.
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Paul Wells