-
Notifications
You must be signed in to change notification settings - Fork 27
Description
Summary
Replace the current Arc<Mutex<State>> with a lock-free AtomicU8 for managing WebRTC substream state, as suggested in PR #513.
This change also proposes merging the two separate wakers (shutdown_waker and write_waker) into a single state_waker. Before implementing, we need to verify this waker consolidation is safe and won't cause issues like infinite poll loops or unacceptable spurious wakeups.
Current Implementation
state: Arc<Mutex<State>>,With separate wakers:
shutdown_waker: Arc<AtomicWaker>- notifies when FIN_ACK receivedwrite_waker: Arc<AtomicWaker>- notifies when STOP_SENDING/RESET_STREAM received
Proposed Implementation
struct SharedState {
// lock-free / more efficient
// BITS 0 to 3
// 0: open 1: sendClosed 2: closing 3: FinSent (our states) 4: FinAck
// BIT 4: remote recv closed via FIN recv
// BIT 5: STOP_SENDING remote send closed
state: AtomicU8,
// Waker for tasks waiting on state changes (ie poll write blocked on state, but poll shutdown has
state_waker: AtomicWaker,
}Investigation Required
Before implementing, we need to verify that we can merge all wakers into a single state_waker:
-
shutdown_waker- currently woken when:- FIN_ACK received (
on_message→State::FinAcked) - Registered in
poll_shutdownwhen waiting for FIN_ACK
- FIN_ACK received (
-
write_waker- currently woken when:- STOP_SENDING received (
on_message→State::SendClosed) - RESET_STREAM received (
on_message→State::SendClosed) - Registered in
poll_writewhen blocked due to backpressure
- STOP_SENDING received (
The key question: Can we safely wake a single state_waker on any state transition without causing spurious wakeups that degrade performance?
Since Substream is owned and polled by exactly one task, merging should be safe - the same task handles both poll_write and poll_shutdown. However, we should verify:
- No scenario where waking causes infinite poll loops
- Performance impact of spurious wakeups is acceptable (state transitions are rare)
Benefits
- Lock-free state access (no mutex contention)
- More efficient for concurrent access patterns
- Reduced memory overhead (single AtomicU8 vs Mutex)
Trade-offs
- Slightly more complex bit manipulation
- May need careful ordering semantics (SeqCst vs Relaxed)
- Single waker approach requires validation
Reference
- webrtc: Support FIN/FIN_ACK handshake for substream shutdown #513
- Comment from @lexnv suggesting the change