Bug 806375: cleanup DataChannel, esp. channel close and connection shutdown r=mcmanus
authorRandell Jesup <rjesup@jesup.org>
Thu, 13 Dec 2012 23:30:11 -0500
changeset 116023 fe5784e9e50bb6eddfa7afda5b4d3e854638ea88
parent 116022 3d898c39d05e155caab76d6635f8ecf4de3ad35a
child 116024 6002b73474a7085819aa342277c86249d524b893
push id19693
push userrjesup@wgate.com
push dateFri, 14 Dec 2012 08:36:42 +0000
treeherdermozilla-inbound@fe5784e9e50b [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmcmanus
bugs806375
milestone20.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 806375: cleanup DataChannel, esp. channel close and connection shutdown r=mcmanus
media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
netwerk/sctp/datachannel/DataChannel.cpp
netwerk/sctp/datachannel/DataChannel.h
--- a/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
+++ b/media/webrtc/signaling/src/peerconnection/PeerConnectionImpl.cpp
@@ -475,20 +475,25 @@ PeerConnectionImpl::ConnectDataConnectio
 #ifdef MOZILLA_INTERNAL_API
   mDataConnection = new mozilla::DataChannelConnection(this);
   NS_ENSURE_TRUE(mDataConnection,NS_ERROR_FAILURE);
   if (!mDataConnection->Init(aLocalport, aNumstreams, true)) {
     CSFLogError(logTag,"%s DataConnection Init Failed",__FUNCTION__);
     return NS_ERROR_FAILURE;
   }
   // XXX Fix! Get the correct flow for DataChannel. Also error handling.
-  nsRefPtr<TransportFlow> flow = mMedia->GetTransportFlow(1,false).get();
-  CSFLogDebugS(logTag, "Transportflow[1] = " << flow.get());
-  if (!mDataConnection->ConnectDTLS(flow, aLocalport, aRemoteport)) {
-    return NS_ERROR_FAILURE;
+  for (int i = 2; i >= 0; i--) {
+    nsRefPtr<TransportFlow> flow = mMedia->GetTransportFlow(i,false).get();
+    CSFLogDebugS(logTag, "Transportflow[" << i << "] = " << flow.get());
+    if (flow) {
+      if (!mDataConnection->ConnectDTLS(flow, aLocalport, aRemoteport)) {
+        return NS_ERROR_FAILURE;
+      }
+      break;
+    }
   }
   return NS_OK;
 #else
     return NS_ERROR_FAILURE;
 #endif
 }
 
 NS_IMETHODIMP
@@ -915,32 +920,35 @@ PeerConnectionImpl::CheckApiState(bool a
   if (mReadyState == kClosed)
     return NS_ERROR_FAILURE;
   return NS_OK;
 }
 
 NS_IMETHODIMP
 PeerConnectionImpl::Close(bool aIsSynchronous)
 {
+  CSFLogDebugS(logTag, __FUNCTION__);
   PC_AUTO_ENTER_API_CALL(false);
 
   return CloseInt(aIsSynchronous);
 }
 
 
 nsresult
 PeerConnectionImpl::CloseInt(bool aIsSynchronous)
 {
   PC_AUTO_ENTER_API_CALL_NO_CHECK();
 
   if (mCall != nullptr)
     mCall->endCall();
 #ifdef MOZILLA_INTERNAL_API
-  if (mDataConnection)
-    mDataConnection->CloseAll();
+  if (mDataConnection) {
+    mDataConnection->Destroy();
+    mDataConnection = nullptr; // it may not go away until the runnables are dead
+  }
 #endif
 
   ShutdownMedia(aIsSynchronous);
 
   // DataConnection will need to stay alive until all threads/runnables exit
 
   return NS_OK;
 }
--- a/netwerk/sctp/datachannel/DataChannel.cpp
+++ b/netwerk/sctp/datachannel/DataChannel.cpp
@@ -41,32 +41,32 @@ GetDataChannelLog()
 }
 #endif
 
 static bool sctp_initialized;
 
 namespace mozilla {
 
 class DataChannelShutdown;
-nsCOMPtr<DataChannelShutdown> gDataChannelShutdown;
+nsRefPtr<DataChannelShutdown> gDataChannelShutdown;
 
 class DataChannelShutdown : public nsIObserver
 {
 public:
   // This needs to be tied to some form object that is guaranteed to be
   // around (singleton likely) unless we want to shutdown sctp whenever
   // we're not using it (and in which case we'd keep a refcnt'd object
   // ref'd by each DataChannelConnection to release the SCTP usrlib via
   // sctp_finish)
 
   NS_DECL_ISUPPORTS
 
   DataChannelShutdown() {}
 
-  void Init() 
+  void Init()
     {
       nsCOMPtr<nsIObserverService> observerService =
         mozilla::services::GetObserverService();
       if (!observerService)
         return;
 
       nsresult rv = observerService->AddObserver(this,
                                                  "profile-change-net-teardown",
@@ -86,16 +86,28 @@ public:
   NS_IMETHODIMP Observe(nsISupports* aSubject, const char* aTopic,
                         const PRUnichar* aData) {
     if (strcmp(aTopic, "profile-change-net-teardown") == 0) {
       LOG(("Shutting down SCTP"));
       if (sctp_initialized) {
         usrsctp_finish();
         sctp_initialized = false;
       }
+      nsCOMPtr<nsIObserverService> observerService =
+        mozilla::services::GetObserverService();
+      if (!observerService)
+        return NS_ERROR_FAILURE;
+
+      nsresult rv = observerService->RemoveObserver(this,
+                                                    "profile-change-net-teardown");
+      MOZ_ASSERT(rv == NS_OK);
+      (void) rv;
+
+      nsRefPtr<DataChannelShutdown> kungFuDeathGrip(this);
+      gDataChannelShutdown = nullptr;
     }
     return NS_OK;
   }
 };
 
 NS_IMPL_ISUPPORTS1(DataChannelShutdown, nsIObserver);
 
 
@@ -135,27 +147,41 @@ DataChannelConnection::DataChannelConnec
   mRemotePort = 0;
   mDeferTimeout = 10;
   mTimerRunning = false;
   LOG(("Constructor DataChannelConnection=%p, listener=%p", this, mListener));
 }
 
 DataChannelConnection::~DataChannelConnection()
 {
-  // XXX Move CloseAll() to a Destroy() call
+  // This may die on the MainThread, or on the STS thread
+  MOZ_ASSERT(mState == CLOSED);
+  MOZ_ASSERT(!mMasterSocket);
+}
+
+void
+DataChannelConnection::Destroy()
+{
   // Though it's probably ok to do this and close the sockets;
   // if we really want it to do true clean shutdowns it can
   // create a dependant Internal object that would remain around
   // until the network shut down the association or timed out.
-  LOG(("Destroying DataChannelConnection"));
+  LOG(("Destroying DataChannelConnection %p", (void *) this));
   CloseAll();
+
   if (mSocket && mSocket != mMasterSocket)
     usrsctp_close(mSocket);
   if (mMasterSocket)
     usrsctp_close(mMasterSocket);
+
+  mSocket = nullptr;
+  mMasterSocket = nullptr;
+
+  // We can't get any more new callbacks from the SCTP library
+  // All existing callbacks have refs to DataChannelConnection
 }
 
 NS_IMPL_THREADSAFE_ISUPPORTS1(DataChannelConnection,
                               nsITimerCallback)
 
 bool
 DataChannelConnection::Init(unsigned short aPort, uint16_t aNumStreams, bool aUsingDtls)
 {
@@ -185,48 +211,61 @@ DataChannelConnection::Init(unsigned sho
 #else
         NS_ASSERTION(!aUsingDtls, "Trying to use SCTP/DTLS without mtransport");
 #endif
       } else {
         LOG(("sctp_init(%d)", aPort));
         usrsctp_init(aPort, nullptr);
       }
 
-      usrsctp_sysctl_set_sctp_debug_on(0 /* SCTP_DEBUG_ALL */);
+      // Set logging to datachannel:6 to get SCTP debugs
+#ifdef PR_LOGGING
+      usrsctp_sysctl_set_sctp_debug_on(GetDataChannelLog()->level > 5 ? SCTP_DEBUG_ALL : 0);
+#endif
       usrsctp_sysctl_set_sctp_blackhole(2);
       sctp_initialized = true;
 
       gDataChannelShutdown = new DataChannelShutdown();
       gDataChannelShutdown->Init();
     }
   }
+
   // XXX FIX! make this a global we get once
   // Find the STS thread
+  nsresult rv;
+  mSTS = do_GetService(NS_SOCKETTRANSPORTSERVICE_CONTRACTID, &rv);
+  MOZ_ASSERT(NS_SUCCEEDED(rv));
 
-  nsresult res;
-  mSTS = do_GetService(NS_SOCKETTRANSPORTSERVICE_CONTRACTID, &res);
-  MOZ_ASSERT(NS_SUCCEEDED(res));
-
-  // Open sctp association across tunnel
+  // Open sctp with a callback
   if ((mMasterSocket = usrsctp_socket(
          aUsingDtls ? AF_CONN : AF_INET,
          SOCK_STREAM, IPPROTO_SCTP, receive_cb, nullptr, 0, this)) == nullptr) {
     return false;
   }
 
   // Make sure when we close the socket, make sure it doesn't call us back again!
   // This would cause it try to use an invalid DataChannelConnection pointer
   struct linger l;
   l.l_onoff = 1;
   l.l_linger = 0;
   if (usrsctp_setsockopt(mMasterSocket, SOL_SOCKET, SO_LINGER,
                          (const void *)&l, (socklen_t)sizeof(struct linger)) < 0) {
     LOG(("Couldn't set SO_LINGER on SCTP socket"));
   }
 
+  // XXX Consider disabling this when we add proper SDP negotiation.
+  // We may want to leave enabled for supporting 'cloning' of SDP offers, which
+  // implies re-use of the same pseudo-port number, or forcing a renegotiation.
+  uint32_t on = 1;
+  if (usrsctp_setsockopt(mMasterSocket, IPPROTO_SCTP, SCTP_REUSE_PORT,
+                         (const void *)&on, (socklen_t)sizeof(on)) < 0) {
+    LOG(("Couldn't set SCTP_REUSE_PORT on SCTP socket"));
+  }
+
+
   if (!aUsingDtls) {
     memset(&encaps, 0, sizeof(encaps));
     encaps.sue_address.ss_family = AF_INET;
     encaps.sue_port = htons(aPort);
     if (usrsctp_setsockopt(mMasterSocket, IPPROTO_SCTP, SCTP_REMOTE_UDP_ENCAPS_PORT,
                            (const void*)&encaps,
                            (socklen_t)sizeof(struct sctp_udpencaps)) < 0) {
       LOG(("*** failed encaps errno %d", errno));
@@ -289,17 +328,17 @@ error_cleanup:
 
 void
 DataChannelConnection::StartDefer()
 {
   nsresult rv;
   if (!NS_IsMainThread()) {
     NS_DispatchToMainThread(new DataChannelOnMessageAvailable(
                             DataChannelOnMessageAvailable::START_DEFER,
-                            this, nullptr));
+                            this, (DataChannel *) nullptr));
     return;
   }
 
   MOZ_ASSERT(NS_IsMainThread());
   if (!mDeferredTimer) {
     mDeferredTimer = do_CreateInstance("@mozilla.org/timer;1", &rv);
     MOZ_ASSERT(mDeferredTimer);
   }
@@ -337,80 +376,87 @@ DataChannelConnection::Notify(nsITimer *
       LOG(("Turned off deferred send timer"));
       mTimerRunning = false;
     }
   }
   return NS_OK;
 }
 
 #ifdef MOZ_PEERCONNECTION
+class DataChannelConnectRunnable : public nsRunnable
+{
+public:
+  DataChannelConnectRunnable(DataChannelConnection *aConnection)
+    : mConnection(aConnection) {}
+
+  NS_IMETHOD Run()
+  {
+    struct sockaddr_conn addr;
+
+    memset(&addr, 0, sizeof(addr));
+    addr.sconn_family = AF_CONN;
+#if !defined(__Userspace_os_Linux) && !defined(__Userspace_os_Windows)
+    addr.sconn_len = sizeof(addr);
+#endif
+    addr.sconn_port = htons(mConnection->mLocalPort);
+
+    int r = usrsctp_bind(mConnection->mMasterSocket, reinterpret_cast<struct sockaddr *>(&addr),
+                         sizeof(addr));
+    if (r < 0) {
+      LOG(("usrsctp_bind failed: %d", r));
+    } else {
+      // This is the remote addr
+      addr.sconn_port = htons(mConnection->mRemotePort);
+      addr.sconn_addr = static_cast<void *>(mConnection.get());
+      r = usrsctp_connect(mConnection->mMasterSocket, reinterpret_cast<struct sockaddr *>(&addr),
+                          sizeof(addr));
+      if (r < 0) {
+        LOG(("usrsctp_connect failed: %d", r));
+      } else {
+        // Notify Connection open
+        LOG(("%s: sending ON_CONNECTION for %p", __FUNCTION__, mConnection.get()));
+        mConnection->mSocket = mConnection->mMasterSocket;
+        mConnection->mState = DataChannelConnection::OPEN;
+        LOG(("DTLS connect() succeeded!  Entering connected mode"));
+
+        NS_DispatchToMainThread(new DataChannelOnMessageAvailable(
+                                  DataChannelOnMessageAvailable::ON_CONNECTION,
+                                  mConnection, true));
+        return NS_OK;
+      }
+    }
+    // on errors, we simply don't notify there was a connection, but we
+    // want to kill the thread (can we kill ourselves here? That would be better)
+    NS_DispatchToMainThread(new DataChannelOnMessageAvailable(
+                              DataChannelOnMessageAvailable::ON_CONNECTION,
+                              mConnection, false));
+    return NS_OK;
+  }
+
+private:
+  nsRefPtr<DataChannelConnection> mConnection;
+};
+
 bool
 DataChannelConnection::ConnectDTLS(TransportFlow *aFlow, uint16_t localport, uint16_t remoteport)
 {
   LOG(("Connect DTLS local %d, remote %d", localport, remoteport));
 
   NS_PRECONDITION(mMasterSocket, "SCTP wasn't initialized before ConnectDTLS!");
   NS_ENSURE_TRUE(aFlow, false);
 
   mTransportFlow = aFlow;
   mTransportFlow->SignalPacketReceived.connect(this, &DataChannelConnection::SctpDtlsInput);
   mLocalPort = localport;
   mRemotePort = remoteport;
 
-  PR_CreateThread(
-    PR_SYSTEM_THREAD,
-    DataChannelConnection::DTLSConnectThread, this,
-    PR_PRIORITY_NORMAL,
-    PR_GLOBAL_THREAD,
-    PR_JOINABLE_THREAD, 0
-  );
-
-  return true; // not finished yet
-}
-
-/* static */
-void
-DataChannelConnection::DTLSConnectThread(void *data)
-{
-  DataChannelConnection *_this = static_cast<DataChannelConnection*>(data);
-  struct sockaddr_conn addr;
-
-  memset(&addr, 0, sizeof(addr));
-  addr.sconn_family = AF_CONN;
-#if defined(__Userspace_os_Darwin)
-  addr.sconn_len = sizeof(addr);
-#endif
-  addr.sconn_port = htons(_this->mLocalPort);
+  nsCOMPtr<nsIRunnable> connect_event = new DataChannelConnectRunnable(this);
+  nsresult rv = NS_NewThread(getter_AddRefs(mConnectThread), connect_event);
 
-  int r = usrsctp_bind(_this->mMasterSocket, reinterpret_cast<struct sockaddr *>(&addr),
-                       sizeof(addr));
-  if (r < 0) {
-    LOG(("usrsctp_bind failed: %d", r));
-    return;
-  }
-
-  // This is the remote addr
-  addr.sconn_port = htons(_this->mRemotePort);
-  addr.sconn_addr = static_cast<void *>(_this);
-  r = usrsctp_connect(_this->mMasterSocket, reinterpret_cast<struct sockaddr *>(&addr),
-                      sizeof(addr));
-  if (r < 0) {
-    LOG(("usrsctp_connect failed: %d", r));
-    return;
-  }
-
-  // Notify Connection open
-  LOG(("%s: sending ON_CONNECTION for %p", __FUNCTION__, _this));
-  _this->mSocket = _this->mMasterSocket;
-  _this->mState = OPEN;
-  LOG(("DTLS connect() succeeded!  Entering connected mode"));
-
-  NS_DispatchToMainThread(new DataChannelOnMessageAvailable(
-                            DataChannelOnMessageAvailable::ON_CONNECTION,
-                            _this, nullptr));
+  return NS_SUCCEEDED(rv);
 }
 
 void
 DataChannelConnection::SctpDtlsInput(TransportFlow *flow,
                                      const unsigned char *data, size_t len)
 {
   //LOG(("%p: SCTP/DTLS received %ld bytes", this, len));
 
@@ -447,18 +493,19 @@ DataChannelConnection::SctpDtlsOutput(vo
     unsigned char *data = new unsigned char[length];
     memcpy(data, buffer, length);
     res = -1;
     // XXX It might be worthwhile to add an assertion against the thread
     // somehow getting into the DataChannel/SCTP code again, as
     // DISPATCH_SYNC is not fully blocking.  This may be tricky, as it
     // needs to be a per-thread check, not a global.
     peer->mSTS->Dispatch(WrapRunnable(
-      peer, &DataChannelConnection::SendPacket, data, length, true
-    ), NS_DISPATCH_NORMAL);
+                           nsRefPtr<DataChannelConnection>(peer),
+                           &DataChannelConnection::SendPacket, data, length, true),
+                         NS_DISPATCH_NORMAL);
     res = 0; // cheat!  Packets can always be dropped later anyways
   }
   return res;
 }
 #endif
 
 // listen for incoming associations
 // Blocks! - Don't call this from main thread!
@@ -505,17 +552,17 @@ DataChannelConnection::Listen(unsigned s
     LOG(("Couldn't set SO_LINGER on SCTP socket"));
   }
 
   // Notify Connection open
   // XXX We need to make sure connection sticks around until the message is delivered
   LOG(("%s: sending ON_CONNECTION for %p", __FUNCTION__, this));
   NS_DispatchToMainThread(new DataChannelOnMessageAvailable(
                             DataChannelOnMessageAvailable::ON_CONNECTION,
-                            this, nullptr));
+                            this, (DataChannel *) nullptr));
   return true;
 }
 
 // Blocks! - Don't call this from main thread!
 bool
 DataChannelConnection::Connect(const char *addr, unsigned short port)
 {
   struct sockaddr_in addr4;
@@ -581,17 +628,17 @@ DataChannelConnection::Connect(const cha
   LOG(("connect() succeeded!  Entering connected mode"));
   mState = OPEN;
 
   // Notify Connection open
   // XXX We need to make sure connection sticks around until the message is delivered
   LOG(("%s: sending ON_CONNECTION for %p", __FUNCTION__, this));
   NS_DispatchToMainThread(new DataChannelOnMessageAvailable(
                             DataChannelOnMessageAvailable::ON_CONNECTION,
-                            this, nullptr));
+                            this, (DataChannel *) nullptr));
   return true;
 }
 
 DataChannel *
 DataChannelConnection::FindChannelByStreamIn(uint16_t streamIn)
 {
   // Auto-extend mStreamsIn as needed
   if (((uint32_t) streamIn) + 1 > mStreamsIn.Length()) {
@@ -971,17 +1018,17 @@ DataChannelConnection::OpenResponseFinis
     mPending.Push(temp);
     // can't notify the user until we can send an OpenResponse
   } else {
     channel->mStreamOut = streamOut;
     mStreamsOut[streamOut] = channel;
     if (SendOpenResponseMessage(streamOut, channel->mStreamIn)) {
       /* Notify ondatachannel */
       // XXX We need to make sure connection sticks around until the message is delivered
-      LOG(("%s: sending ON_CHANNEL_CREATED for %s: %d/%d", __FUNCTION__, 
+      LOG(("%s: sending ON_CHANNEL_CREATED for %s: %d/%d", __FUNCTION__,
            channel->mLabel.get(), streamOut, channel->mStreamIn));
       NS_DispatchToMainThread(new DataChannelOnMessageAvailable(
                                 DataChannelOnMessageAvailable::ON_CHANNEL_CREATED,
                                 this, channel));
     } else {
       if (errno == EAGAIN || errno == EWOULDBLOCK) {
         channel->mFlags |= DATA_CHANNEL_FLAGS_SEND_RSP;
         StartDefer();
@@ -1388,35 +1435,39 @@ DataChannelConnection::HandleSendFailedE
 }
 
 void
 DataChannelConnection::ResetOutgoingStream(uint16_t streamOut)
 {
   uint32_t i;
 
   mLock.AssertCurrentThreadOwns();
-  LOG(("Resetting outgoing stream %d",streamOut));
+  LOG(("Connection %p: Resetting outgoing stream %d",
+       (void *) this, streamOut));
   // Rarely has more than a couple items and only for a short time
   for (i = 0; i < mStreamsResetting.Length(); ++i) {
     if (mStreamsResetting[i] == streamOut) {
       return;
     }
   }
   mStreamsResetting.AppendElement(streamOut);
 }
 
 void
 DataChannelConnection::SendOutgoingStreamReset()
 {
   struct sctp_reset_streams *srs;
   uint32_t i;
   size_t len;
 
+  LOG(("Connection %p: Sending outgoing stream reset for %d streams",
+       (void *) this, mStreamsResetting.Length()));
   mLock.AssertCurrentThreadOwns();
-  if (mStreamsResetting.IsEmpty() == 0) {
+  if (mStreamsResetting.IsEmpty()) {
+    LOG(("No streams to reset"));
     return;
   }
   len = sizeof(sctp_assoc_t) + (2 + mStreamsResetting.Length()) * sizeof(uint16_t);
   srs = static_cast<struct sctp_reset_streams *> (moz_xmalloc(len)); // infallible malloc
   memset(srs, 0, len);
   srs->srs_flags = SCTP_STREAM_RESET_OUTGOING;
   srs->srs_number_streams = mStreamsResetting.Length();
   for (i = 0; i < mStreamsResetting.Length(); ++i) {
@@ -1438,50 +1489,67 @@ DataChannelConnection::HandleStreamReset
 
   if (!(strrst->strreset_flags & SCTP_STREAM_RESET_DENIED) &&
       !(strrst->strreset_flags & SCTP_STREAM_RESET_FAILED)) {
     n = (strrst->strreset_length - sizeof(struct sctp_stream_reset_event)) / sizeof(uint16_t);
     for (i = 0; i < n; ++i) {
       if (strrst->strreset_flags & SCTP_STREAM_RESET_INCOMING_SSN) {
         channel = FindChannelByStreamIn(strrst->strreset_stream_list[i]);
         if (channel) {
-          LOG(("Channel %d outgoing/%d incoming closed",
-               channel->mStreamOut,channel->mStreamIn));
-          mStreamsIn[channel->mStreamIn] = nullptr;
-          channel->mStreamIn = INVALID_STREAM;
-          if (channel->mStreamOut == INVALID_STREAM) {
-            channel->mPrPolicy = SCTP_PR_SCTP_NONE;
-            channel->mPrValue = 0;
-            channel->mFlags = 0;
-            channel->mState = CLOSED;
+          // The other side closed the channel
+          // We could be in three states:
+          // 1. Normal state (input and output streams (OPEN)
+          //    Notify application, send a RESET in response on our
+          //    outbound channel.  Go to CLOSED
+          // 2. We sent our own reset (CLOSING); either they crossed on the
+          //    wire, or this is a response to our Reset.
+          //    Go to CLOSED
+          // 3. We've sent a open but haven't gotten a response yet (OPENING)
+          //    I believe this is impossible, as we don't have an input stream yet.
+
+          LOG(("Incoming: Channel %d outgoing/%d incoming closed, state %d",
+               channel->mStreamOut, channel->mStreamIn, channel->mState));
+          MOZ_ASSERT(channel->mState == OPEN || channel->mState == CLOSING);
+          if (channel->mState == OPEN) {
+            ResetOutgoingStream(channel->mStreamOut);
             NS_DispatchToMainThread(new DataChannelOnMessageAvailable(
                                       DataChannelOnMessageAvailable::ON_CHANNEL_CLOSED, this,
                                       channel));
-          } else {
-            ResetOutgoingStream(channel->mStreamOut);
-            channel->mState = CLOSING;
+            mStreamsOut[channel->mStreamOut] = nullptr;
           }
+          mStreamsIn[channel->mStreamIn] = nullptr;
+
+          LOG(("Disconnected DataChannel %p from connection %p",
+               (void *) channel.get(), (void *) channel->mConnection.get()));
+          channel->Destroy();
+          // At this point when we leave here, the object is a zombie held alive only by the DOM object
+        } else {
+          LOG(("Can't find incoming channel %d",i));
         }
       }
+
       if (strrst->strreset_flags & SCTP_STREAM_RESET_OUTGOING_SSN) {
         channel = FindChannelByStreamOut(strrst->strreset_stream_list[i]);
-        if (channel != nullptr && channel->mStreamOut != INVALID_STREAM) {
-          LOG(("Channel %d outgoing/%d incoming closed",
-               channel->mStreamOut,channel->mStreamIn));
-          mStreamsOut[channel->mStreamOut] = nullptr;
-          channel->mStreamOut = INVALID_STREAM;
-          if (channel->mStreamIn == INVALID_STREAM) {
-            channel->mPrPolicy = SCTP_PR_SCTP_NONE;
-            channel->mPrValue = 0;
-            channel->mFlags = 0;
-            channel->mState = CLOSED;
-            NS_DispatchToMainThread(new DataChannelOnMessageAvailable(
-                                      DataChannelOnMessageAvailable::ON_CHANNEL_CLOSED, this,
-                                      channel));
+        if (channel) {
+          LOG(("Outgoing: Connection %p channel %p  streams: %d outgoing/%d incoming closed",
+               (void *) this, (void *) channel.get(), channel->mStreamOut, channel->mStreamIn));
+
+          MOZ_ASSERT(channel->mState == CLOSING);
+          if (channel->mState == CLOSING) {
+            mStreamsOut[channel->mStreamOut] = nullptr;
+            if (channel->mStreamIn != INVALID_STREAM)
+              mStreamsIn[channel->mStreamIn] = nullptr;
+            LOG(("Disconnected DataChannel %p from connection %p (refcnt will be %u)",
+                 (void *) channel.get(), (void *) channel->mConnection.get(),
+                 (uint32_t) channel->mConnection->mRefCnt-1));
+            channel->Destroy();
+            // At this point when we leave here, the object is a zombie held alive only by the DOM object
           }
+        } else {
+          LOG(("Can't find outgoing channel %d",i));
         }
       }
     }
   }
 }
 
 void
 DataChannelConnection::HandleStreamChangeEvent(const struct sctp_stream_change_event *strchg)
@@ -1686,17 +1754,17 @@ DataChannelConnection::Open(const nsACSt
       prPolicy = SCTP_PR_SCTP_TTL;
       break;
   }
   if ((prPolicy == SCTP_PR_SCTP_NONE) && (prValue != 0)) {
     return nullptr;
   }
 
   flags = !inOrder ? DATA_CHANNEL_FLAG_OUT_OF_ORDER_ALLOWED : 0;
-  nsRefPtr<DataChannel> channel(new DataChannel(this, 
+  nsRefPtr<DataChannel> channel(new DataChannel(this,
                                                 INVALID_STREAM, INVALID_STREAM,
                                                 DataChannel::CONNECTING,
                                                 label, type, prValue,
                                                 flags,
                                                 aListener, aContext));
 
   MutexAutoLock lock(mLock); // OpenFinish assumes this
   return OpenFinish(channel.forget());
@@ -1933,67 +2001,83 @@ DataChannelConnection::SendMsgCommon(uin
 }
 
 void
 DataChannelConnection::Close(uint16_t streamOut)
 {
   nsRefPtr<DataChannel> channel; // make sure it doesn't go away on us
 
   MutexAutoLock lock(mLock);
-  LOG(("Closing stream %d",streamOut));
   channel = FindChannelByStreamOut(streamOut);
   if (channel) {
+    LOG(("Connection %p/Channel %p: Closing stream %d",
+         (void *) channel->mConnection.get(), (void *) channel.get(), streamOut));
     channel->mBufferedData.Clear();
     if (channel->mStreamOut != INVALID_STREAM)
       ResetOutgoingStream(channel->mStreamOut);
     SendOutgoingStreamReset();
     channel->mState = CLOSING;
+  } else {
+    LOG(("!!!? no channel when closing stream %d?",streamOut));
   }
 }
 
 void DataChannelConnection::CloseAll()
 {
   LOG(("Closing all channels"));
   // Don't need to lock here
 
   // Make sure no more channels will be opened
   mState = CLOSED;
 
   // Close current channels
-  // FIX! if there are runnables, they must use weakrefs or hold a strong
-  // ref and keep the channel and/or connection alive
+  // If there are runnables, they hold a strong ref and keep the channel
+  // and/or connection alive (even if in a CLOSED state)
   for (uint32_t i = 0; i < mStreamsOut.Length(); ++i) {
     if (mStreamsOut[i]) {
       mStreamsOut[i]->Close();
     }
   }
 
   // Clean up any pending opens for channels
   nsRefPtr<DataChannel> channel;
   while (nullptr != (channel = dont_AddRef(static_cast<DataChannel *>(mPending.PopFront()))))
     channel->Close(); // also releases the ref on each iteration
 }
 
 DataChannel::~DataChannel()
 {
+  if (mConnection)
+    Close();
+}
+
+// Used when disconnecting from the DataChannelConnection
+void
+DataChannel::Destroy()
+{
   LOG(("Destroying Data channel %d/%d", mStreamOut, mStreamIn));
-  Close();
+  MOZ_ASSERT_IF(mStreamOut != INVALID_STREAM,
+                !mConnection->FindChannelByStreamOut(mStreamOut));
+  MOZ_ASSERT_IF(mStreamIn != INVALID_STREAM,
+                !mConnection->FindChannelByStreamIn(mStreamIn));
+  mStreamIn  = INVALID_STREAM;
+  mStreamOut = INVALID_STREAM;
+  mState = CLOSED;
+  mConnection = nullptr;
 }
 
 void
 DataChannel::Close()
-{ 
+{
   if (mState == CLOSING || mState == CLOSED ||
       mStreamOut == INVALID_STREAM) {
     return;
   }
   mState = CLOSING;
   mConnection->Close(mStreamOut);
-  mStreamOut = INVALID_STREAM;
-  mStreamIn  = INVALID_STREAM;
 }
 
 void
 DataChannel::SetListener(DataChannelListener *aListener, nsISupports *aContext)
 {
   MOZ_ASSERT(!mListener); // only should be set once, avoids races w/o locking
   mContext = aContext;
   mListener = aListener;
@@ -2031,9 +2115,8 @@ DataChannel::GetBufferedAmount()
   uint32_t buffered = 0;
   for (uint32_t i = 0; i < mBufferedData.Length(); ++i) {
     buffered += mBufferedData[i]->mLength;
   }
   return buffered;
 }
 
 } // namespace mozilla
-
--- a/netwerk/sctp/datachannel/DataChannel.h
+++ b/netwerk/sctp/datachannel/DataChannel.h
@@ -104,16 +104,17 @@ public:
     // Called when a new DataChannel has been opened by the other side.
     virtual void NotifyDataChannel(DataChannel *channel) = 0;
   };
 
   DataChannelConnection(DataConnectionListener *listener);
   virtual ~DataChannelConnection();
 
   bool Init(unsigned short aPort, uint16_t aNumStreams, bool aUsingDtls);
+  void Destroy(); // So we can spawn refs tied to runnables in shutdown
 
   // These block; they require something to decide on listener/connector
   // (though you can do simultaneous Connect()).  Do not call these from
   // the main thread!
   bool Listen(unsigned short port);
   bool Connect(const char *addr, unsigned short port);
 
 #ifdef SCTP_DTLS_SUPPORTED
@@ -163,16 +164,18 @@ public:
   friend class DataChannel;
   Mutex  mLock;
 
 protected:
   friend class DataChannelOnMessageAvailable;
   DataConnectionListener *mListener;
 
 private:
+  friend class DataChannelConnectRunnable;
+
 #ifdef SCTP_DTLS_SUPPORTED
   static void DTLSConnectThread(void *data);
   int SendPacket(const unsigned char* data, size_t len, bool release);
   void SctpDtlsInput(TransportFlow *flow, const unsigned char *data, size_t len);
   static int SctpDtlsOutput(void *addr, void *buffer, size_t length, uint8_t tos, uint8_t set_df);
 #endif
   DataChannel* FindChannelByStreamIn(uint16_t streamIn);
   DataChannel* FindChannelByStreamOut(uint16_t streamOut);
@@ -230,31 +233,34 @@ private:
   // channels available from the stack must be negotiated!
   nsAutoTArray<nsRefPtr<DataChannel>,16> mStreamsOut;
   nsAutoTArray<nsRefPtr<DataChannel>,16> mStreamsIn;
   nsDeque mPending; // Holds already_AddRefed<DataChannel>s -- careful!
 
   // Streams pending reset
   nsAutoTArray<uint16_t,4> mStreamsResetting;
 
-  struct socket *mMasterSocket;
-  struct socket *mSocket;
-  uint16_t mState;
+  struct socket *mMasterSocket; // accessed from connect thread
+  struct socket *mSocket; // cloned from mMasterSocket on successful Connect on connect thread
+  uint16_t mState; // modified on connect thread (to OPEN)
 
 #ifdef SCTP_DTLS_SUPPORTED
   nsRefPtr<TransportFlow> mTransportFlow;
   nsCOMPtr<nsIEventTarget> mSTS;
 #endif
-  uint16_t mLocalPort;
+  uint16_t mLocalPort; // Accessed from connect thread
   uint16_t mRemotePort;
 
   // Timer to control when we try to resend blocked messages
   nsCOMPtr<nsITimer> mDeferredTimer;
   uint32_t mDeferTimeout; // in ms
   bool mTimerRunning;
+
+  // Thread used for connections
+  nsCOMPtr<nsIThread> mConnectThread;
 };
 
 class DataChannel {
 public:
   enum {
     CONNECTING = 0U,
     OPEN = 1U,
     CLOSING = 2U,
@@ -281,16 +287,17 @@ public:
     , mPrValue(value)
     , mFlags(0)
     , mContext(aContext)
     {
       NS_ASSERTION(mConnection,"NULL connection");
     }
 
   ~DataChannel();
+  void Destroy(); // when we disconnect from the connection after stream RESET
 
   NS_INLINE_DECL_THREADSAFE_REFCOUNTING(DataChannel)
 
   // Close this DataChannel.  Can be called multiple times.
   void Close();
 
   // Set the listener (especially for channels created from the other side)
   // Note: The Listener and Context should only be set once
@@ -408,16 +415,24 @@ public:
 
   DataChannelOnMessageAvailable(int32_t     aType,
                                 DataChannelConnection *aConnection,
                                 DataChannel *aChannel)
     : mType(aType),
       mChannel(aChannel),
       mConnection(aConnection) {}
 
+  // for ON_CONNECTION
+  DataChannelOnMessageAvailable(int32_t     aType,
+                                DataChannelConnection *aConnection,
+                                bool aResult)
+    : mType(aType),
+      mConnection(aConnection),
+      mResult(aResult) {}
+
   NS_IMETHOD Run()
   {
     switch (mType) {
       case ON_DATA:
       case ON_CHANNEL_OPEN:
       case ON_CHANNEL_CLOSED:
         if (!mChannel->mListener)
           return NS_OK;
@@ -444,17 +459,20 @@ public:
         break;
       case ON_CHANNEL_CLOSED:
         mChannel->mListener->OnChannelClosed(mChannel->mContext);
         break;
       case ON_CHANNEL_CREATED:
         mConnection->mListener->NotifyDataChannel(mChannel);
         break;
       case ON_CONNECTION:
-        mConnection->mListener->NotifyConnection();
+        if (mResult) {
+          mConnection->mListener->NotifyConnection();
+        }
+        mConnection->mConnectThread = nullptr; // kill the connection thread
         break;
       case ON_DISCONNECTED:
         mConnection->mListener->NotifyClosedConnection();
         break;
       case START_DEFER:
         mConnection->StartDefer();
         break;
     }
@@ -465,13 +483,14 @@ private:
   ~DataChannelOnMessageAvailable() {}
 
   int32_t                           mType;
   // XXX should use union
   nsRefPtr<DataChannel>             mChannel;
   nsRefPtr<DataChannelConnection>   mConnection;
   nsCString                         mData;
   int32_t                           mLen;
+  bool                              mResult;
 };
 
 }
 
 #endif  // NETWERK_SCTP_DATACHANNEL_DATACHANNEL_H_