From 4182b499b39418376fff7c9a68e3aa457ea68c37 Mon Sep 17 00:00:00 2001 From: Taylor Brandstetter Date: Tue, 20 Oct 2020 17:33:03 -0700 Subject: [PATCH] Avoid duplicate usrsctp_init if the last usrsctp_finish failed. When deinitializing usrsctp, we attempt to call usrsctp_finish in a loop for three seconds (it may fail because another sctp thread is holding a reference to something). If the three seconds run out, usrsctp is left in a still initialized state, and bad things happen down the road if usrsctp_init is called in the state. Bug: chromium:1138878 Change-Id: I9c24d51d5a274b06bdf4183261694fc2989136c5 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/189940 Reviewed-by: Harald Alvestrand Commit-Queue: Taylor Cr-Commit-Position: refs/heads/master@{#32467} --- media/sctp/sctp_transport.cc | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/media/sctp/sctp_transport.cc b/media/sctp/sctp_transport.cc index a74c66431c..2b2cc42f05 100644 --- a/media/sctp/sctp_transport.cc +++ b/media/sctp/sctp_transport.cc @@ -58,6 +58,7 @@ static constexpr size_t kSctpMtu = 1200; // Set the initial value of the static SCTP Data Engines reference count. ABSL_CONST_INIT int g_usrsctp_usage_count = 0; +ABSL_CONST_INIT bool g_usrsctp_initialized_ = false; ABSL_CONST_INIT webrtc::GlobalMutex g_usrsctp_lock_(absl::kConstInit); // DataMessageType is used for the SCTP "Payload Protocol Identifier", as @@ -262,9 +263,19 @@ class SctpTransport::UsrSctpWrapper { public: static void InitializeUsrSctp() { RTC_LOG(LS_INFO) << __FUNCTION__; - // First argument is udp_encapsulation_port, which is not releveant for our - // AF_CONN use of sctp. - usrsctp_init(0, &UsrSctpWrapper::OnSctpOutboundPacket, &DebugSctpPrintf); + // UninitializeUsrSctp tries to call usrsctp_finish in a loop for three + // seconds; if that failed and we were left in a still-initialized state, we + // don't want to call usrsctp_init again as that will result in undefined + // behavior. + if (g_usrsctp_initialized_) { + RTC_LOG(LS_WARNING) << "Not reinitializing usrsctp since last attempt at " + "usrsctp_finish failed."; + } else { + // First argument is udp_encapsulation_port, which is not releveant for + // our AF_CONN use of sctp. + usrsctp_init(0, &UsrSctpWrapper::OnSctpOutboundPacket, &DebugSctpPrintf); + g_usrsctp_initialized_ = true; + } // To turn on/off detailed SCTP debugging. You will also need to have the // SCTP_DEBUG cpp defines flag, which can be turned on in media/BUILD.gn. @@ -317,6 +328,7 @@ class SctpTransport::UsrSctpWrapper { // closed. Wait and try again until it succeeds for up to 3 seconds. for (size_t i = 0; i < 300; ++i) { if (usrsctp_finish() == 0) { + g_usrsctp_initialized_ = false; delete g_transport_map_; g_transport_map_ = nullptr; return;