Unsynchronized read of packet_queue in _get_available_packet_count() races the network-thread writer #198

Summary

WebRTCLibDataChannel::_get_available_packet_count() reads packet_queue.size() without taking mutex, while every other accessor of packet_queue (queue_packet(), _get_packet()) does take it. On an unordered, maxRetransmits=0 channel under real network loss/reorder, this unsynchronized read can appear to "stick" at a stale value (observed: 0) for a sustained span of engine frames even while the peer confirms packets are still arriving, because the reader on the game thread and the writer on libdatachannel's network thread are racing on the same std::queue with no memory-visibility guarantee between them.

We hit this as an application-level stall (movement-acknowledgement freeze) in a Godot 4.7 C# game and were able to isolate it down to this function via source review + a live dual-tcpdump trace. Filing honestly: we have one confirmed mechanism (below) and a second, unconfirmed one we want to flag rather than quietly drop, in case it's useful to someone who knows this code better than we do.

Environment

• webrtc-native 1.2.1-stable (Linux x86_64, both debug and release templates)
• Vendored libdatachannel 0.24.5 (per that release's own dependency pin)
• Godot 4.7.stable.mono, C#/.NET 8
• Linux (the same behavior does not depend on anything Linux-specific in the code path we traced — flagging OS for completeness, not as a suspected factor)
• Confirmed the relevant file is byte-identical between 1.2.1-stable and current master (src/WebRTCLibDataChannel.cpp), so this is still live on master as of this report.

The confirmed mechanism

src/WebRTCLibDataChannel.cpp:

void WebRTCLibDataChannel::queue_packet(const uint8_t *data, uint32_t size, bool p_is_string) {
	mutex->lock();
	std::vector<uint8_t> packet;
	packet.resize(size);
	memcpy(&packet[0], data, size);
	packet_queue.push(QueuedPacket(packet, p_is_string));
	mutex->unlock();
}

(line ~101-110; called from the libdatachannel channel's onMessage callback, i.e. on libdatachannel's own network thread)

Error WebRTCLibDataChannel::_get_packet(const uint8_t **r_buffer, int32_t *r_len) {
	ERR_FAIL_COND_V(packet_queue.empty(), ERR_UNAVAILABLE);
	mutex->lock();
	current_packet = packet_queue.front();
	packet_queue.pop();
	...
	mutex->unlock();
	return OK;
}

(line ~181-196; called from the game thread via _poll/get_packet on the WebRTCDataChannel API)

int32_t WebRTCLibDataChannel::_get_available_packet_count() const {
	return packet_queue.size();
}

(line ~216-218; called from the game thread, e.g. every frame from GDScript/C# to decide whether to drain the channel)

_get_available_packet_count() reads the same packet_queue queue_packet() mutates, from a different thread, with no lock and no atomic/relaxed-consistency guarantee. (As a smaller aside: _get_packet()'s own packet_queue.empty() check on line 182 also reads the queue before taking the lock on line 184 — same class of unsynchronized read, just less consequential since the guarded section below it re-checks nothing and would only misbehave on the empty→nonempty edge.)

We don't have a from-first-principles proof of the specific compiler/memory-model mechanism (whether it's a genuine data race per the C++ standard causing UB, a stale-cache-line staleness that eventually resolves, or something else) — we're reporting the structural asymmetry (one accessor locked, the other not) plus a reproducible symptom that's consistent with it.

The symptom that led us here

Our game runs one unordered, maxRetransmits=0 WebRTC data channel per direction for high-frequency (60Hz-ish) authoritative game state, specifically because we don't want head-of-line blocking from retransmits on stale samples. Under a netem-shaped profile that is fairly mild by videogame-networking standards — 120ms RTT, 2% round-trip packet loss, ~30ms jitter — we could reliably reproduce multi-tick "freezes" where:

• The client's own _get_available_packet_count() (surfaced to us via Godot's WebRTCDataChannel.GetAvailablePacketCount()) reads 0 for a sustained span (measured: as long as ~71 consecutive 60Hz engine frames, ~1.2s, in our worst captured window).
• A concurrent tcpdump on the client's network interface, capturing the raw UDP/DTLS datagrams (we can't decrypt DTLS without an SSLKEYLOGFILE-equivalent, which this extension doesn't expose, but datagram arrival timing/size is visible unencrypted at the UDP layer) shows datagrams physically arriving at the client during that exact window, at a cadence consistent with the server's actual send rate — i.e., not consistent with the datagrams simply having been lost on the wire.

That combination — data demonstrably at the socket, but the extension's own exposed packet count not reflecting it for over a second of wall-clock time — is what pointed us at an app-level read/write race rather than network loss as the cause of the stall. We worked around it on our side by moving our specific piece of state (a monotonic sequence acknowledgement) onto a separate reliable/ordered channel, which sidesteps this channel's _get_available_packet_count() entirely; we are not proposing that as an upstream fix, just noting it's why we aren't going to be able to easily push a "still repros in isolation" build further ourselves right now.

Suggested fix (untested by us against this codebase — offered as a starting point, not a vetted patch)

Either:

• take mutex in _get_available_packet_count() (cheapest, matches the existing pattern used everywhere else packet_queue is touched), or
• replace the plain size_t/std::queue::size() read with an std::atomic<size_t> maintained alongside the queue by both queue_packet() and _get_packet(), if lock-contention on a hot per-frame read is a concern.

What we could NOT confirm (flagging rather than omitting)

We also instrumented the reliable/ordered control channel client-side, the same way, to see whether it shows the same freeze signature (which would point at something further upstream — e.g. in libdatachannel itself or the SCTP/DTLS demux — rather than this specific unsynchronized read, which is unordered-channel-specific by nature of how we're using it). One rep out of four showed a data point suggestive of continued control-channel drain during a movement-channel freeze window, which would argue against a shared root cause — but our control channel's own baseline message rate (~1 per 177ms) is too sparse relative to the ~116ms freeze windows we were measuring for that single data point to be statistically meaningful either way. We are not claiming the control channel is unaffected; we're saying we don't have evidence strong enough to claim anything about it, and didn't want to manufacture false confidence by citing a single underpowered rep as a finding.

Happy to provide the raw pcaps/traces or build a minimal standalone repro (two headless Godot processes + tc netem) if that would help track this down — didn't want to over-invest in that before hearing whether this is already understood/expected behavior we're missing something about.