In SctpDataChannel use plain bool as safety flag.

SctpDataChannel can be destroyed on various threads and thus safety flag
couldn't be properly invalidated in the destructor. Instead of relying
on it when posting async send, rely on reference counting of the full
object

Bug: chromium:504716948
Change-Id: I1246fecac26d0761be006b069658d072183546bf
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/469980
Reviewed-by: Tomas Gunnarsson <tommi@webrtc.org>
Commit-Queue: Danil Chapovalov <danilchap@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#47635}

pc/sctp_data_channel.cc

diff --git a/pc/sctp_data_channel.cc b/pc/sctp_data_channel.cc
index a7338aa..6d7305a 100644
--- a/pc/sctp_data_channel.cc
+++ b/pc/sctp_data_channel.cc
@@ -344,16 +344,14 @@
       negotiated_(config.negotiated),
       ordered_(config.ordered),
       observer_(nullptr),
-      controller_(std::move(controller)) {
+      controller_(std::move(controller)),
+      connected_to_transport_(connected_to_transport) {
   RTC_DCHECK_RUN_ON(network_thread_);
   // Since we constructed on the network thread we can't (yet) check the
   // `controller_` pointer since doing so will trigger a thread check.
   RTC_UNUSED(network_thread_);
   RTC_DCHECK(config.IsValid());
 
-  if (connected_to_transport)
-    network_safety_->SetAlive();
-
   switch (config.open_handshake_role) {
     case InternalDataChannelInit::kNone:  // pre-negotiated
       handshake_state_ = kHandshakeReady;
@@ -611,14 +609,17 @@
   // thread. So we always post to the network thread (even if the current thread
   // might be the network thread - in theory a call could even come from within
   // the `on_complete` callback).
-  network_thread_->PostTask(SafeTask(
-      network_safety_, [this, buffer = std::move(buffer),
-                        on_complete = std::move(on_complete)]() mutable {
-        RTC_DCHECK_RUN_ON(network_thread_);
-        RTCError err = SendImpl(std::move(buffer));
-        if (on_complete)
-          std::move(on_complete)(err);
-      }));
+  scoped_refptr<SctpDataChannel> me(this);
+  network_thread_->PostTask([me = std::move(me), buffer = std::move(buffer),
+                             on_complete = std::move(on_complete)]() mutable {
+    RTC_DCHECK_RUN_ON(me->network_thread_);
+    if (!me->connected_to_transport()) {
+      return;
+    }
+    RTCError err = me->SendImpl(std::move(buffer));
+    if (on_complete)
+      std::move(on_complete)(err);
+  });
 }
 
 void SctpDataChannel::SetSctpSid_n(StreamId sid) {
@@ -661,7 +662,7 @@
 
 void SctpDataChannel::OnTransportChannelCreated() {
   RTC_DCHECK_RUN_ON(network_thread_);
-  network_safety_->SetAlive();
+  connected_to_transport_ = true;
 }
 
 void SctpDataChannel::OnTransportChannelClosed(RTCError error) {
@@ -782,7 +783,7 @@
     return;
   }
 
-  network_safety_->SetNotAlive();
+  connected_to_transport_ = false;
 
   // Still go to "kClosing" before "kClosed", since observers may be expecting
   // that.

pc/sctp_data_channel.h

diff --git a/pc/sctp_data_channel.h b/pc/sctp_data_channel.h
index 2ac0a3e..693661b 100644
--- a/pc/sctp_data_channel.h
+++ b/pc/sctp_data_channel.h
@@ -273,7 +273,7 @@
       RTC_RUN_ON(network_thread_);
 
   bool connected_to_transport() const RTC_RUN_ON(network_thread_) {
-    return network_safety_->alive();
+    return connected_to_transport_;
   }
   void MaybeSendOnBufferedAmountChanged() RTC_RUN_ON(network_thread_);
 
@@ -306,9 +306,8 @@
       kHandshakeInit;
   // Did we already start the graceful SCTP closing procedure?
   bool started_closing_procedure_ RTC_GUARDED_BY(network_thread_) = false;
+  bool connected_to_transport_ RTC_GUARDED_BY(network_thread_) = false;
   PacketQueue queued_received_data_ RTC_GUARDED_BY(network_thread_);
-  scoped_refptr<PendingTaskSafetyFlag> network_safety_ =
-      PendingTaskSafetyFlag::CreateDetachedInactive();
 };
 
 }  // namespace webrtc

pc/sctp_data_channel_unittest.cc

diff --git a/pc/sctp_data_channel_unittest.cc b/pc/sctp_data_channel_unittest.cc
index 06b4a88..c4e30ee 100644
--- a/pc/sctp_data_channel_unittest.cc
+++ b/pc/sctp_data_channel_unittest.cc
@@ -613,6 +613,22 @@
   EXPECT_EQ(RTCErrorDetailType::SCTP_FAILURE, channel_->error().error_detail());
 }
 
+TEST_F(SctpDataChannelTest, ChannelDeletedWhileDataBuffered) {
+  AddObserver();
+  SetChannelReady();
+
+  CopyOnWriteBuffer buffer(100 * 1024);
+  memset(buffer.MutableData(), 0, buffer.size());
+  DataBuffer packet(buffer, true);
+
+  // Send a very large packet, forcing the message to become buffered.
+  channel_->SendAsync(packet, nullptr);
+
+  // Delete the channel, expect no crashes.
+  inner_channel_ = nullptr;
+  channel_ = nullptr;
+}
+
 TEST_F(SctpDataChannelTest, TransportGotErrorCode) {
   SetChannelReady();