fix(peer_connection): implement SCTP/DataChannel close on PeerConnection shutdown

rtc/src/peer_connection/handler/datachannel.rs

@@ -11,7 +11,7 @@ use crate::statistics::accumulator::RTCStatsAccumulator;
 use log::{debug, warn};
 use sctp::PayloadProtocolIdentifier;
 use shared::TransportContext;
-use shared::error::{Error, Result};
+use shared::error::{Error, Result, flatten_errs};
 use std::collections::{HashMap, VecDeque};
 use std::time::Instant;
 
@@ -329,6 +329,16 @@ impl<'a> sansio::Protocol<TaggedRTCMessageInternal, TaggedRTCMessageInternal, RT
     }
 
     fn close(&mut self) -> Result<()> {
-        Ok(())
+        let mut errs = vec![];
+        for dc in self.data_channels.values_mut() {
+            if let Err(e) = dc.close() {
+                errs.push(e);
+            }
+        }
+        // Do NOT clear data_channels here: public RTCDataChannel methods
+        // (label(), ready_state(), etc.) use unwrap() on map lookups, so
+        // clearing the map would panic if the application still holds a
+        // DataChannel handle after PeerConnection::close().
+        flatten_errs(errs)
     }
 }

rtc/src/peer_connection/handler/mod.rs

@@ -378,63 +378,87 @@ where
         // https://www.w3.org/TR/webrtc/#dom-rtcpeerconnection-close (step #3)
         self.signaling_state = RTCSignalingState::Closed;
 
-        // Try closing everything and collect the errors
-        // Shutdown strategy:
-        // 1. All Conn close by closing their underlying Conn.
-        // 2. A Mux stops this chain. It won't close the underlying
-        //    Conn if one of the endpoints is closed down. To
-        //    continue the chain the Mux has to be closed.
-        for_each_handler!(forward: process_handler!(self, handler, {
-            handler.close()?;
+        // Close handlers in reverse order (Endpoint→Interceptor→SRTP→DataChannel→SCTP→
+        // DTLS→ICE→Demuxer) so that higher-level protocols (DataChannels, SCTP) shut down
+        // before the transports they depend on (DTLS, ICE).
+        // Collect all errors instead of short-circuiting so every handler gets a chance
+        // to clean up even if an earlier one fails.
+        let mut close_errs: Vec<Error> = vec![];
+        for_each_handler!(reverse: process_handler!(self, handler, {
+            if let Err(e) = handler.close() {
+                close_errs.push(e);
+            }
         }));
 
-        let close_errs: Vec<Error> = vec![];
+        // W3C WebRTC §close shutdown work is implemented in individual handler close() methods:
+        //   InterceptorHandler::close() → interceptor.close()
+        //   DataChannelHandler::close() → closes all RTCDataChannelInternal instances
+        //   SctpHandler::close()        → closes all SCTP associations + endpoint
+        //   DtlsHandler::close()        → dtls_transport.stop()
+        //   IceHandler::close()         → ice_transport.agent.close()
 
-        /* TODO:
-        if let Err(err) = self.interceptor.close().await {
-            close_errs.push(Error::new(format!("interceptor: {err}")));
-        }
+        self.update_connection_state(true);
 
-        // https://www.w3.org/TR/webrtc/#dom-rtcpeerconnection-close (step #4)
-        {
-            let mut rtp_transceivers = self.internal.rtp_transceivers.lock().await;
-            for t in &*rtp_transceivers {
-                if let Err(err) = t.stop().await {
-                    close_errs.push(Error::new(format!("rtp_transceivers: {err}")));
-                }
-            }
-            rtp_transceivers.clear();
-        }
+        flatten_errs(close_errs)
+    }
+}
 
-        // https://www.w3.org/TR/webrtc/#dom-rtcpeerconnection-close (step #5)
-        {
-            let mut data_channels = self.internal.sctp_transport.data_channels.lock().await;
-            for d in &*data_channels {
-                if let Err(err) = d.close().await {
-                    close_errs.push(Error::new(format!("data_channels: {err}")));
-                }
-            }
-            data_channels.clear();
-        }
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use crate::peer_connection::RTCPeerConnectionBuilder;
+    use sansio::Protocol;
 
-        // https://www.w3.org/TR/webrtc/#dom-rtcpeerconnection-close (step #6)
-        if let Err(err) = self.internal.sctp_transport.stop().await {
-            close_errs.push(Error::new(format!("sctp_transport: {err}")));
-        }
+    #[test]
+    fn close_transitions_to_closed_state() {
+        let mut pc = RTCPeerConnectionBuilder::new().build().unwrap();
+        assert_ne!(pc.peer_connection_state, RTCPeerConnectionState::Closed);
+        assert_ne!(pc.signaling_state, RTCSignalingState::Closed);
 
-        // https://www.w3.org/TR/webrtc/#dom-rtcpeerconnection-close (step #7)
-        if let Err(err) = self.internal.dtls_transport.stop().await {
-            close_errs.push(Error::new(format!("dtls_transport: {err}")));
-        }
+        pc.close().unwrap();
 
-        // https://www.w3.org/TR/webrtc/#dom-rtcpeerconnection-close (step #8, #9, #10)
-        if let Err(err) = self.internal.ice_transport.stop().await {
-            close_errs.push(Error::new(format!("ice_transport: {err}")));
-        }
-         */
+        assert_eq!(pc.peer_connection_state, RTCPeerConnectionState::Closed);
+        assert_eq!(pc.signaling_state, RTCSignalingState::Closed);
+    }
 
-        self.update_connection_state(true);
+    #[test]
+    fn close_is_idempotent() {
+        let mut pc = RTCPeerConnectionBuilder::new().build().unwrap();
+        pc.close().unwrap();
+        // Second close should be a no-op (early return for already-closed)
+        pc.close().unwrap();
+        assert_eq!(pc.peer_connection_state, RTCPeerConnectionState::Closed);
+    }
 
-        flatten_errs(close_errs)
+    #[test]
+    fn close_preserves_data_channels_in_map() {
+        let mut pc = RTCPeerConnectionBuilder::new().build().unwrap();
+        // Create a data channel before closing
+        let dc = pc.create_data_channel("test", None).unwrap();
+        let dc_id = dc.id();
+        assert!(pc.data_channels.contains_key(&dc_id));
+
+        pc.close().unwrap();
+
+        // Data channels must remain in the map so that public RTCDataChannel
+        // methods (label(), ready_state(), etc.) which use unwrap() don't panic.
+        assert!(
+            pc.data_channels.contains_key(&dc_id),
+            "data_channels map must not be cleared on close"
+        );
+    }
+
+    #[test]
+    fn close_with_data_channel_does_not_panic() {
+        let mut pc = RTCPeerConnectionBuilder::new().build().unwrap();
+        let dc = pc.create_data_channel("my-dc", None).unwrap();
+        let dc_id = dc.id();
+        pc.close().unwrap();
+
+        // After close, public accessors on the data channel must still work
+        // (they unwrap() the map entry).
+        let dc = pc.data_channel(dc_id).unwrap();
+        assert_eq!(dc.label(), "my-dc");
+        let _ = dc.ready_state();
     }
 }

rtc/src/peer_connection/handler/sctp.rs

@@ -12,8 +12,8 @@ use datachannel::message::Message;
 use datachannel::message::message_channel_threshold::DataChannelThreshold;
 use log::{debug, warn};
 use sctp::{
-    AssociationEvent, AssociationHandle, ClientConfig, DatagramEvent, EndpointEvent, Event,
-    Payload, PayloadProtocolIdentifier, StreamEvent,
+    AssociationError, AssociationEvent, AssociationHandle, ClientConfig, DatagramEvent,
+    EndpointEvent, Event, Payload, PayloadProtocolIdentifier, StreamEvent,
 };
 use shared::error::{Error, Result};
 use shared::marshal::Unmarshal;
@@ -467,7 +467,18 @@ impl<'a> sansio::Protocol<TaggedRTCMessageInternal, TaggedRTCMessageInternal, RT
     }
 
     fn close(&mut self) -> Result<()> {
-        Ok(())
+        // Close all SCTP associations and drop the endpoint.
+        let sctp = &mut self.ctx.sctp_transport;
+        let mut errs = vec![];
+        for assoc in sctp.sctp_associations.values_mut() {
+            if let Err(e) = assoc.close(AssociationError::LocallyClosed) {
+                errs.push(e);
+            }
+        }
+        sctp.sctp_associations.clear();
+        sctp.sctp_endpoint = None;
+
+        shared::error::flatten_errs(errs)
     }
 }