Bug 1584751 - Only open DataChannelConnections that aren't already open; r=bwc
authorNico Grunbaum <na-g@nostrum.com>
Sat, 05 Oct 2019 02:48:15 +0000
changeset 496454 7bbdfe6958a4305ea430db9d217771a5230c1cd5
parent 496453 363625fae57859e7311acd89164e4789a24f4a0c
child 496455 93aec03cc20257ce02982d4599471fe9015c2eb5
child 496614 dbe05c7204a38b7df389c42ca1d022ff94a51b53
push id97250
push userna-g@nostrum.com
push dateSun, 06 Oct 2019 01:36:18 +0000
treeherderautoland@7bbdfe6958a4 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbwc
bugs1584751
milestone71.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 1584751 - Only open DataChannelConnections that aren't already open; r=bwc Differential Revision: https://phabricator.services.mozilla.com/D47691
netwerk/sctp/datachannel/DataChannel.cpp
netwerk/sctp/datachannel/DataChannel.h
--- a/netwerk/sctp/datachannel/DataChannel.cpp
+++ b/netwerk/sctp/datachannel/DataChannel.cpp
@@ -640,41 +640,72 @@ void DataChannelConnection::SetMaxMessag
             aMaxMessageSize != mMaxMessageSize ? "yes" : "no"));
 }
 
 uint64_t DataChannelConnection::GetMaxMessageSize() { return mMaxMessageSize; }
 
 #ifdef MOZ_PEERCONNECTION
 
 bool DataChannelConnection::ConnectToTransport(const std::string& aTransportId,
-                                               bool aClient, uint16_t localport,
-                                               uint16_t remoteport) {
-  {
-    MutexAutoLock lock(mLock);
-
-    DC_DEBUG(("Connect DTLS local %u, remote %u", localport, remoteport));
-
-    MOZ_ASSERT(mMasterSocket,
-               "SCTP wasn't initialized before ConnectToTransport!");
-    if (NS_WARN_IF(aTransportId.empty())) {
-      return false;
+                                               const bool aClient,
+                                               const uint16_t aLocalPort,
+                                               const uint16_t aRemotePort) {
+  MutexAutoLock lock(mLock);
+
+  MOZ_ASSERT(mMasterSocket,
+             "SCTP wasn't initialized before ConnectToTransport!");
+  static const auto paramString =
+      [](const std::string& tId, const Maybe<bool>& client,
+         const uint16_t localPort, const uint16_t remotePort) -> std::string {
+    std::ostringstream stream;
+    stream << "Transport ID: '" << tId << "', Role: '"
+           << (client ? (client.value() ? "client" : "server") : "")
+           << "', Local Port: '" << localPort << "', Remote Port: '"
+           << remotePort << "'";
+    return stream.str();
+  };
+
+  const auto params =
+      paramString(aTransportId, Some(aClient), aLocalPort, aRemotePort);
+  DC_DEBUG(("ConnectToTransport connecting DTLS transport with parameters: %s",
+            params.c_str()));
+
+  const auto currentReadyState = GetReadyState();
+  if (currentReadyState == OPEN) {
+    if (aTransportId == mTransportId && mAllocateEven.isSome() &&
+        mAllocateEven.value() == aClient && mLocalPort == aLocalPort &&
+        mRemotePort == aRemotePort) {
+      DC_WARN(
+          ("Skipping attempt to connect to an already OPEN transport with "
+           "identical parameters."));
+      return true;
     }
-
-    mLocalPort = localport;
-    mRemotePort = remoteport;
-    SetReadyState(CONNECTING);
-    mAllocateEven = Some(aClient);
-
-    // Could be faster. Probably doesn't matter.
-    while (auto channel = mChannels.Get(INVALID_STREAM)) {
-      mChannels.Remove(channel);
-      channel->mStream = FindFreeStream();
-      if (channel->mStream != INVALID_STREAM) {
-        mChannels.Insert(channel);
-      }
+    DC_WARN(
+        ("Attempting to connect to an already OPEN transport, because "
+         "different parameters were provided."));
+    DC_WARN(("Original transport parameters: %s",
+             paramString(mTransportId, mAllocateEven, mLocalPort, aRemotePort)
+                 .c_str()));
+    DC_WARN(("New transport parameters: %s", params.c_str()));
+  }
+  if (NS_WARN_IF(aTransportId.empty())) {
+    return false;
+  }
+
+  mLocalPort = aLocalPort;
+  mRemotePort = aRemotePort;
+  SetReadyState(CONNECTING);
+  mAllocateEven = Some(aClient);
+
+  // Could be faster. Probably doesn't matter.
+  while (auto channel = mChannels.Get(INVALID_STREAM)) {
+    mChannels.Remove(channel);
+    channel->mStream = FindFreeStream();
+    if (channel->mStream != INVALID_STREAM) {
+      mChannels.Insert(channel);
     }
   }
   RUN_ON_THREAD(mSTS,
                 WrapRunnable(RefPtr<DataChannelConnection>(this),
                              &DataChannelConnection::SetSignals, aTransportId),
                 NS_DISPATCH_NORMAL);
   return true;
 }
--- a/netwerk/sctp/datachannel/DataChannel.h
+++ b/netwerk/sctp/datachannel/DataChannel.h
@@ -152,18 +152,19 @@ class DataChannelConnection final : publ
   // 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);
 #endif
 
 #ifdef SCTP_DTLS_SUPPORTED
-  bool ConnectToTransport(const std::string& aTransportId, bool aClient,
-                          uint16_t localport, uint16_t remoteport);
+  bool ConnectToTransport(const std::string& aTransportId, const bool aClient,
+                          const uint16_t aLocalPort,
+                          const uint16_t aRemotePort);
   void TransportStateChange(const std::string& aTransportId,
                             TransportLayer::State aState);
   void CompleteConnect();
   void SetSignals(const std::string& aTransportId);
 #endif
 
   typedef enum {
     RELIABLE = 0,