Backed out changesets ba08d4e02e0c and b729ddecf1d1 (bug 1400563) for bustage. FIREFOX_RELEASE_57_BASE
authorRyan VanderMeulen <ryanvm@gmail.com>
Wed, 01 Nov 2017 22:41:37 -0400
changeset 692964 14581cf06e7fb0556dffebed82fe81b21e1fc68d
parent 692963 eb82b9fadf9edad7dc7de1755ec4670bcd334858
child 692965 b7c372f8a26c510c30a0e49b89801508cb2116dc
push id87647
push userbmo:jkt@mozilla.com
push dateFri, 03 Nov 2017 18:34:26 +0000
bugs1400563
milestone57.0
backs outba08d4e02e0c3839aa85985c7fcdf68bba94dac5
b729ddecf1d1807b64705588702d2d70b6cc87d8
Backed out changesets ba08d4e02e0c and b729ddecf1d1 (bug 1400563) for bustage.
netwerk/sctp/datachannel/DataChannel.cpp
netwerk/sctp/datachannel/DataChannel.h
--- a/netwerk/sctp/datachannel/DataChannel.cpp
+++ b/netwerk/sctp/datachannel/DataChannel.cpp
@@ -41,17 +41,16 @@
 #include "mozilla/Sprintf.h"
 #include "nsProxyRelease.h"
 #include "nsThread.h"
 #include "nsThreadUtils.h"
 #include "nsAutoPtr.h"
 #include "nsNetUtil.h"
 #include "nsNetCID.h"
 #include "mozilla/StaticPtr.h"
-#include "mozilla/StaticMutex.h"
 #include "mozilla/Unused.h"
 #ifdef MOZ_PEERCONNECTION
 #include "mtransport/runnable_utils.h"
 #endif
 
 #define DATACHANNEL_LOG(args) LOG(args)
 #include "DataChannel.h"
 #include "DataChannelProtocol.h"
@@ -67,53 +66,25 @@ static bool sctp_initialized;
 
 namespace mozilla {
 
 LazyLogModule gDataChannelLog("DataChannel");
 static LazyLogModule gSCTPLog("SCTP");
 
 #define SCTP_LOG(args) MOZ_LOG(mozilla::gSCTPLog, mozilla::LogLevel::Debug, args)
 
-class DataChannelConnectionShutdown : public nsITimerCallback
-{
-public:
-  explicit DataChannelConnectionShutdown(DataChannelConnection* aConnection)
-    : mConnection(aConnection)
-  {
-    mTimer = NS_NewTimer(); // we'll crash if this fails
-    mTimer->InitWithCallback(this, 30*1000, nsITimer::TYPE_ONE_SHOT);
-  }
-
-  NS_IMETHODIMP Notify(nsITimer* aTimer) override;
-
-  NS_DECL_THREADSAFE_ISUPPORTS
-
-private:
-  virtual ~DataChannelConnectionShutdown()
-  {
-    mTimer->Cancel();
-  }
-
-  RefPtr<DataChannelConnection> mConnection;
-  nsCOMPtr<nsITimer> mTimer;
-};
-
-class DataChannelShutdown;
-
-StaticRefPtr<DataChannelShutdown> sDataChannelShutdown;
-
 class DataChannelShutdown : public nsIObserver
 {
 public:
-  // This needs to be tied to some object that is guaranteed to be
+  // 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). Right now, the single instance of this class is
-  // owned by the observer service and a StaticRefPtr.
+  // owned by the observer service.
 
   NS_DECL_ISUPPORTS
 
   DataChannelShutdown() {}
 
   void Init()
     {
       nsCOMPtr<nsIObserverService> observerService =
@@ -123,88 +94,47 @@ public:
 
       nsresult rv = observerService->AddObserver(this,
                                                  "xpcom-will-shutdown",
                                                  false);
       MOZ_ASSERT(rv == NS_OK);
       (void) rv;
     }
 
+private:
+  // The only instance of DataChannelShutdown is owned by the observer
+  // service, so there is no need to call RemoveObserver here.
+  virtual ~DataChannelShutdown() = default;
+
+public:
   NS_IMETHOD Observe(nsISupports* aSubject, const char* aTopic,
-                     const char16_t* aData) override
-  {
+                     const char16_t* aData) override {
     // Note: MainThread
     if (strcmp(aTopic, "xpcom-will-shutdown") == 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,
                                                     "xpcom-will-shutdown");
       MOZ_ASSERT(rv == NS_OK);
       (void) rv;
-
-      {
-        StaticMutexAutoLock lock(sLock);
-        sConnections = nullptr; // clears as well
-      }
-      sDataChannelShutdown = nullptr;
     }
     return NS_OK;
   }
-
-  void CreateConnectionShutdown(DataChannelConnection* aConnection)
-  {
-    StaticMutexAutoLock lock(sLock);
-    if (!sConnections) {
-      sConnections = new nsTArray<RefPtr<DataChannelConnectionShutdown>>();
-    }
-    sConnections->AppendElement(new DataChannelConnectionShutdown(aConnection));
-  }
-
-  void RemoveConnectionShutdown(DataChannelConnectionShutdown* aConnectionShutdown)
-  {
-    StaticMutexAutoLock lock(sLock);
-    if (sConnections) {
-      sConnections->RemoveElement(aConnectionShutdown);
-    }
-  }
-
-private:
-  // The only instance of DataChannelShutdown is owned by the observer
-  // service, so there is no need to call RemoveObserver here.
-  virtual ~DataChannelShutdown() = default;
-
-  // protects sConnections
-  static StaticMutex sLock;
-  static StaticAutoPtr<nsTArray<RefPtr<DataChannelConnectionShutdown>>> sConnections;
 };
 
-StaticMutex DataChannelShutdown::sLock;
-StaticAutoPtr<nsTArray<RefPtr<DataChannelConnectionShutdown>>> DataChannelShutdown::sConnections;
-
 NS_IMPL_ISUPPORTS(DataChannelShutdown, nsIObserver);
 
-NS_IMPL_ISUPPORTS(DataChannelConnectionShutdown, nsITimerCallback)
-
-NS_IMETHODIMP
-DataChannelConnectionShutdown::Notify(nsITimer* aTimer)
-{
-  // safely release reference to ourself
-  RefPtr<DataChannelConnectionShutdown> grip(this);
-  sDataChannelShutdown->RemoveConnectionShutdown(this);
-  return NS_OK;
-}
-
 OutgoingMsg::OutgoingMsg(struct sctp_sendv_spa &info, const uint8_t *data,
                          size_t length)
   : mLength(length)
   , mData(data)
 {
   mInfo = &info;
   mPos = 0;
 }
@@ -317,22 +247,26 @@ DataChannelConnection::DataChannelConnec
 
 DataChannelConnection::~DataChannelConnection()
 {
   LOG(("Deleting DataChannelConnection %p", (void *) this));
   // This may die on the MainThread, or on the STS thread
   ASSERT_WEBRTC(mState == CLOSED);
   MOZ_ASSERT(!mMasterSocket);
   MOZ_ASSERT(mPending.GetSize() == 0);
-  MOZ_ASSERT(!mTransportFlow);
 
   // Already disconnected from sigslot/mTransportFlow
   // TransportFlows must be released from the STS thread
   if (!IsSTSThread()) {
     ASSERT_WEBRTC(NS_IsMainThread());
+    if (mTransportFlow) {
+      ASSERT_WEBRTC(mSTS);
+      NS_ProxyRelease(
+        "DataChannelConnection::mTransportFlow", mSTS, mTransportFlow.forget());
+    }
 
     if (mInternalIOThread) {
       // Avoid spinning the event thread from here (which if we're mainthread
       // is in the event loop already)
       nsCOMPtr<nsIRunnable> r = WrapRunnable(nsCOMPtr<nsIThread>(mInternalIOThread),
                                              &nsIThread::Shutdown);
       Dispatch(r.forget());
     }
@@ -387,29 +321,16 @@ void DataChannelConnection::DestroyOnSTS
     usrsctp_close(aSocket);
   if (aMasterSocket)
     usrsctp_close(aMasterSocket);
 
   usrsctp_deregister_address(static_cast<void *>(this));
   LOG(("Deregistered %p from the SCTP stack.", static_cast<void *>(this)));
 
   disconnect_all();
-
-  // we may have queued packet sends on STS after this; dispatch to ourselves before finishing here
-  // so we can be sure there aren't anymore runnables active that can try to touch the flow.
-  // DON'T use RUN_ON_THREAD, it queue-jumps!
-  mSTS->Dispatch(WrapRunnable(RefPtr<DataChannelConnection>(this),
-                              &DataChannelConnection::DestroyOnSTSFinal),
-                 NS_DISPATCH_NORMAL);
-}
-
-void DataChannelConnection::DestroyOnSTSFinal()
-{
-  mTransportFlow = nullptr;
-  sDataChannelShutdown->CreateConnectionShutdown(this);
 }
 
 bool
 DataChannelConnection::Init(unsigned short aPort, uint16_t aNumStreams, bool aMaxMessageSizeSet,
                             uint64_t aMaxMessageSize)
 {
   struct sctp_initmsg initmsg;
   struct sctp_assoc_value av;
@@ -458,18 +379,18 @@ DataChannelConnection::Init(unsigned sho
       usrsctp_sysctl_set_sctp_ecn_enable(0);
 
       // Enable interleaving messages for different streams (incoming)
       // See: https://tools.ietf.org/html/rfc6458#section-8.1.20
       usrsctp_sysctl_set_sctp_default_frag_interleave(2);
 
       sctp_initialized = true;
 
-      sDataChannelShutdown = new DataChannelShutdown();
-      sDataChannelShutdown->Init();
+      RefPtr<DataChannelShutdown> shutdown = new DataChannelShutdown();
+      shutdown->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));
--- a/netwerk/sctp/datachannel/DataChannel.h
+++ b/netwerk/sctp/datachannel/DataChannel.h
@@ -145,17 +145,16 @@ public:
 
   bool Init(unsigned short aPort, uint16_t aNumStreams, bool aMaxMessageSizeSet,
             uint64_t aMaxMessageSize);
 
   void Destroy(); // So we can spawn refs tied to runnables in shutdown
   // Finish Destroy on STS to avoid SCTP race condition with ABORT from far end
   void DestroyOnSTS(struct socket *aMasterSocket,
                     struct socket *aSocket);
-  void DestroyOnSTSFinal();
 
   void SetMaxMessageSize(bool aMaxMessageSizeSet, uint64_t aMaxMessageSize);
   uint64_t GetMaxMessageSize();
 
 #ifdef ALLOW_DIRECT_SCTP_LISTEN_CONNECT
   // These block; they require something to decide on listener/connector
   // (though you can do simultaneous Connect()).  Do not call these from
   // the main thread!