webrtc: Remove test timeout and add atomic state issue doc

Author: timwu20

ATOMIC_STATE_ISSUE.md

@@ -0,0 +1,70 @@
+# Replace Mutex<State> with lock-free AtomicU8 and consolidate wakers for WebRTC substream
+
+## 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
+
+```rust
+state: Arc<Mutex<State>>,
+```
+
+With separate wakers:
+- `shutdown_waker: Arc<AtomicWaker>` - notifies when FIN_ACK received
+- `write_waker: Arc<AtomicWaker>` - notifies when STOP_SENDING/RESET_STREAM received
+
+## Proposed Implementation
+
+```rust
+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`**:
+
+1. **`shutdown_waker`** - currently woken when:
+   - FIN_ACK received (`on_message` → `State::FinAcked`)
+   - Registered in `poll_shutdown` when waiting for FIN_ACK
+
+2. **`write_waker`** - currently woken when:
+   - STOP_SENDING received (`on_message` → `State::SendClosed`)
+   - RESET_STREAM received (`on_message` → `State::SendClosed`)
+   - Registered in `poll_write` when blocked due to backpressure
+
+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
+
+- PR #513: https://github.com/paritytech/litep2p/pull/513
+- Comment from @lexnv suggesting the change

src/transport/webrtc/substream.rs

@@ -41,13 +41,8 @@ const MAX_FRAME_SIZE: usize = 16384;
 
 /// Timeout for waiting on FIN_ACK after sending FIN.
 /// Matches go-libp2p's 5 second stream close timeout.
-#[cfg(not(test))]
 const FIN_ACK_TIMEOUT: Duration = Duration::from_secs(5);
 
-/// Shorter timeout for tests.
-#[cfg(test)]
-const FIN_ACK_TIMEOUT: Duration = Duration::from_secs(2);
-
 /// Substream event.
 #[derive(Debug, PartialEq, Eq)]
 pub enum Event {