Bug 1231971 - Refactor the NAT simulator to use e10s sockets when appropriate. r=drno

media/mtransport/nr_socket_prsock.cpp

@@ -909,6 +909,8 @@ int NrSocket::listen(int backlog) {
   assert(fd_);
   status = PR_Listen(fd_, backlog);
   if (status != PR_SUCCESS) {
+    r_log(LOG_GENERIC, LOG_CRIT, "%s: PR_GetError() == %d",
+          __FUNCTION__, PR_GetError());
     ABORT(R_IO_ERROR);
   }
 
@@ -2055,24 +2057,26 @@ static nr_socket_vtbl nr_socket_local_vtbl={
   nr_socket_local_accept
 };
 
-int nr_socket_local_create(void *obj, nr_transport_addr *addr, nr_socket **sockp) {
+/* static */
-  RefPtr<NrSocketBase> sock;
+int
+NrSocketBase::CreateSocket(nr_transport_addr *addr, RefPtr<NrSocketBase> *sock)
+{
   int r, _status;
 
   // create IPC bridge for content process
   if (XRE_IsParentProcess()) {
-    sock = new NrSocket();
+    *sock = new NrSocket();
   } else {
     switch (addr->protocol) {
       case IPPROTO_UDP:
-        sock = new NrUdpSocketIpc();
+        *sock = new NrUdpSocketIpc();
         break;
       case IPPROTO_TCP:
 #if defined(MOZILLA_INTERNAL_API) && !defined(MOZILLA_XPCOMRT_API)
         {
           nsCOMPtr<nsIThread> main_thread;
           NS_GetMainThread(getter_AddRefs(main_thread));
-          sock = new NrTcpSocketIpc(main_thread.get());
+          *sock = new NrTcpSocketIpc(main_thread.get());
         }
 #else
         ABORT(R_REJECTED);
@@ -2081,10 +2085,27 @@ int nr_socket_local_create(void *obj, nr_transport_addr *addr, nr_socket **sockp
     }
   }
 
-  r = sock->create(addr);
+  r = (*sock)->create(addr);
   if (r)
     ABORT(r);
 
+  _status = 0;
+abort:
+  if (_status) {
+    *sock = nullptr;
+  }
+  return _status;
+}
+
+int nr_socket_local_create(void *obj, nr_transport_addr *addr, nr_socket **sockp) {
+  RefPtr<NrSocketBase> sock;
+  int r, _status;
+
+  r = NrSocketBase::CreateSocket(addr, &sock);
+  if (r) {
+    ABORT(r);
+  }
+
   r = nr_socket_create_int(static_cast<void *>(sock),
                            sock->vtbl(), sockp);
   if (r)

media/mtransport/nr_socket_prsock.h

@@ -96,6 +96,10 @@ public:
   }
   virtual ~NrSocketBase() {}
 
+  // Factory method; will create either an NrSocket, NrUdpSocketIpc, or
+  // NrTcpSocketIpc as appropriate.
+  static int CreateSocket(nr_transport_addr *addr, RefPtr<NrSocketBase> *sock);
+
   // the nr_socket APIs
   virtual int create(nr_transport_addr *addr) = 0;
   virtual int sendto(const void *msg, size_t len,
@@ -132,9 +136,10 @@ public:
     return my_addr_;
   }
 
-protected:
   void fire_callback(int how);
 
+protected:
+
   bool connect_invoked_;
   nr_transport_addr my_addr_;
 

media/mtransport/test/ice_unittest.cpp

@@ -2098,7 +2098,7 @@ TEST_F(IceGatherTest, TestFakeStunServerNoNatDefaultRouteOnly) {
 
 TEST_F(IceGatherTest, TestStunTcpServerTrickle) {
   UseFakeStunTcpServerWithResponse("192.0.3.1", 3333);
-  TestStunServer::GetInstance(AF_INET)->SetDelay(500);
+  TestStunTcpServer::GetInstance(AF_INET)->SetDelay(500);
   Gather(0);
   ASSERT_FALSE(StreamHasMatchingCandidate(0, " 192.0.3.1 ", " tcptype "));
   WaitForGather();

media/mtransport/test/test_nr_socket_unittest.cpp

@@ -33,6 +33,8 @@ extern "C" {
 #include "gtest/gtest.h"
 #include "gtest_utils.h"
 
+#define DATA_BUF_SIZE 1024
+
 namespace mozilla {
 
 class TestNrSocketTest : public ::testing::Test {
@@ -62,12 +64,13 @@ class TestNrSocketTest : public ::testing::Test {
   }
 
   RefPtr<TestNrSocket> CreateTestNrSocket_s(const char *ip_str,
-                                              TestNat *nat) {
+                                            int proto,
+                                            TestNat *nat) {
     // If no nat is supplied, we create a default NAT which is disabled. This
     // is how we simulate a non-natted socket.
     RefPtr<TestNrSocket> sock(new TestNrSocket(nat ? nat : new TestNat));
     nr_transport_addr address;
-    nr_str_port_to_transport_addr(ip_str, 0, IPPROTO_UDP, &address);
+    nr_str_port_to_transport_addr(ip_str, 0, proto, &address);
     int r = sock->create(&address);
     if (r) {
       return nullptr;
@@ -75,40 +78,47 @@ class TestNrSocketTest : public ::testing::Test {
     return sock;
   }
 
-  void CreatePublicAddrs(size_t count, const char *ip_str = "127.0.0.1") {
+  void CreatePublicAddrs(size_t count,
+                         const char *ip_str = "127.0.0.1",
+                         int proto = IPPROTO_UDP) {
     sts_->Dispatch(
         WrapRunnable(this,
                      &TestNrSocketTest::CreatePublicAddrs_s,
                      count,
-                     ip_str),
+                     ip_str,
+                     proto),
         NS_DISPATCH_SYNC);
   }
 
-  void CreatePublicAddrs_s(size_t count, const char* ip_str) {
+  void CreatePublicAddrs_s(size_t count, const char* ip_str, int proto) {
     while (count--) {
-      auto sock = CreateTestNrSocket_s(ip_str, nullptr);
+      auto sock = CreateTestNrSocket_s(ip_str, proto, nullptr);
       ASSERT_TRUE(sock) << "Failed to create socket";
       public_addrs_.push_back(sock);
     }
   }
 
   RefPtr<TestNat> CreatePrivateAddrs(size_t size,
-                                       const char* ip_str = "127.0.0.1") {
+                                     const char* ip_str = "127.0.0.1",
+                                     int proto = IPPROTO_UDP) {
     RefPtr<TestNat> result;
     sts_->Dispatch(
         WrapRunnableRet(&result,
                         this,
                         &TestNrSocketTest::CreatePrivateAddrs_s,
                         size,
-                        ip_str),
+                        ip_str,
+                        proto),
         NS_DISPATCH_SYNC);
     return result;
   }
 
-  RefPtr<TestNat> CreatePrivateAddrs_s(size_t count, const char* ip_str) {
+  RefPtr<TestNat> CreatePrivateAddrs_s(size_t count,
+                                       const char* ip_str,
+                                       int proto) {
     RefPtr<TestNat> nat(new TestNat);
     while (count--) {
-      auto sock = CreateTestNrSocket_s(ip_str, nat);
+      auto sock = CreateTestNrSocket_s(ip_str, proto, nat);
       if (!sock) {
         EXPECT_TRUE(false) << "Failed to create socket";
         break;
@@ -178,6 +188,63 @@ class TestNrSocketTest : public ::testing::Test {
                                 sender_external_address);
   }
 
+  bool CheckTcpConnectivity(TestNrSocket *from, TestNrSocket *to) {
+    NrSocketBase *accepted_sock;
+    if (!Connect(from, to, &accepted_sock)) {
+      std::cerr << "Connect failed" << std::endl;
+      return false;
+    }
+
+    // write on |from|, recv on |accepted_sock|
+    if (!WaitForWriteable(from)) {
+      std::cerr << __LINE__ << "WaitForWriteable failed" << std::endl;
+      return false;
+    }
+
+    int r;
+    sts_->Dispatch(WrapRunnableRet(&r,
+                                   this,
+                                   &TestNrSocketTest::SendDataTcp_s,
+                                   from),
+                   NS_DISPATCH_SYNC);
+    if (r) {
+      std::cerr << "SendDataTcp_s (1) failed" << std::endl;
+      return false;
+    }
+
+    sts_->Dispatch(WrapRunnableRet(&r,
+                                   this,
+                                   &TestNrSocketTest::RecvDataTcp_s,
+                                   accepted_sock),
+                   NS_DISPATCH_SYNC);
+    if (r) {
+      std::cerr << "RecvDataTcp_s (1) failed" << std::endl;
+      return false;
+    }
+
+    sts_->Dispatch(WrapRunnableRet(&r,
+                                   this,
+                                   &TestNrSocketTest::SendDataTcp_s,
+                                   accepted_sock),
+                   NS_DISPATCH_SYNC);
+    if (r) {
+      std::cerr << "SendDataTcp_s (2) failed" << std::endl;
+      return false;
+    }
+
+    sts_->Dispatch(WrapRunnableRet(&r,
+                                   this,
+                                   &TestNrSocketTest::RecvDataTcp_s,
+                                   from),
+                   NS_DISPATCH_SYNC);
+    if (r) {
+      std::cerr << "RecvDataTcp_s (2) failed" << std::endl;
+      return false;
+    }
+
+    return true;
+  }
+
   int GetAddress(TestNrSocket *sock, nr_transport_addr_ *address) {
     MOZ_ASSERT(sock);
     MOZ_ASSERT(address);
@@ -203,10 +270,16 @@ class TestNrSocketTest : public ::testing::Test {
         const_cast<nr_transport_addr*>(&to));
   }
 
+  int SendDataTcp_s(NrSocketBase *from) {
+    // It is up to caller to ensure that |from| is writeable.
+    const char buf[] = "foobajooba";
+    size_t written;
+    return from->write(buf, sizeof(buf), &written);
+  }
+
   int RecvData_s(TestNrSocket *to, nr_transport_addr *from) {
     // It is up to caller to ensure that |to| is readable
-    const size_t bufSize = 1024;
+    char buf[DATA_BUF_SIZE];
-    char buf[bufSize];
     size_t len;
     // Maybe check that data matches?
     int r = to->recvfrom(buf, sizeof(buf), &len, 0, from);
@@ -216,6 +289,100 @@ class TestNrSocketTest : public ::testing::Test {
     return r;
   }
 
+  int RecvDataTcp_s(NrSocketBase *to) {
+    // It is up to caller to ensure that |to| is readable
+    char buf[DATA_BUF_SIZE];
+    size_t len;
+    // Maybe check that data matches?
+    int r = to->read(buf, sizeof(buf), &len);
+    if (!r && (len == 0)) {
+      r = R_INTERNAL;
+    }
+    return r;
+  }
+
+  int Listen_s(TestNrSocket *to) {
+    // listen on |to|
+    int r = to->listen(1);
+    if (r) {
+      return r;
+    }
+    return 0;
+  }
+
+  int Connect_s(TestNrSocket *from, TestNrSocket *to) {
+    // connect on |from|
+    nr_transport_addr destination_address;
+    int r = to->getaddr(&destination_address);
+    if (r) {
+      return r;
+    }
+
+    r = from->connect(&destination_address);
+    if (r) {
+      return r;
+    }
+
+    return 0;
+  }
+
+  int Accept_s(TestNrSocket *to, NrSocketBase **accepted_sock) {
+    nr_socket *sock;
+    nr_transport_addr source_address;
+    int r = to->accept(&source_address, &sock);
+    if (r) {
+      return r;
+    }
+
+    *accepted_sock = reinterpret_cast<NrSocketBase*>(sock->obj);
+    return 0;
+  }
+
+  bool Connect(TestNrSocket *from,
+               TestNrSocket *to,
+               NrSocketBase **accepted_sock) {
+    int r;
+    sts_->Dispatch(WrapRunnableRet(&r,
+                                   this,
+                                   &TestNrSocketTest::Listen_s,
+                                   to),
+                   NS_DISPATCH_SYNC);
+    if (r) {
+      std::cerr << "Listen_s failed: " << r << std::endl;
+      return false;
+    }
+
+    sts_->Dispatch(WrapRunnableRet(&r,
+                                   this,
+                                   &TestNrSocketTest::Connect_s,
+                                   from,
+                                   to),
+                   NS_DISPATCH_SYNC);
+    if (r && r != R_WOULDBLOCK) {
+      std::cerr << "Connect_s failed: " << r << std::endl;
+      return false;
+    }
+
+    if (!WaitForReadable(to)) {
+      std::cerr << "WaitForReadable failed" << std::endl;
+      return false;
+    }
+
+    sts_->Dispatch(WrapRunnableRet(&r,
+                                   this,
+                                   &TestNrSocketTest::Accept_s,
+                                   to,
+                                   accepted_sock),
+                   NS_DISPATCH_SYNC);
+
+    if (r) {
+      std::cerr << "Accept_s failed: " << r << std::endl;
+      return false;
+    }
+    return true;
+  }
+
+
   bool WaitForSocketState(TestNrSocket *sock, int state) {
     MOZ_ASSERT(sock);
     sts_->Dispatch(WrapRunnable(this,
@@ -641,8 +808,52 @@ TEST_F(TestNrSocketTest, FullConeTimeout) {
                                     sender_external_address));
 }
 
-// TODO(): We need TCP tests, but first we will need ICE TCP to land (this
+TEST_F(TestNrSocketTest, PublicConnectivityTcp)
-// adds listen/accept support to NrSocket)
+{
+  CreatePublicAddrs(2, "127.0.0.1", IPPROTO_TCP);
+
+  ASSERT_TRUE(CheckTcpConnectivity(public_addrs_[0], public_addrs_[1]));
+}
+
+TEST_F(TestNrSocketTest, PrivateConnectivityTcp) {
+  RefPtr<TestNat> nat(CreatePrivateAddrs(2, "127.0.0.1", IPPROTO_TCP));
+  nat->filtering_type_ = TestNat::ENDPOINT_INDEPENDENT;
+  nat->mapping_type_ = TestNat::ENDPOINT_INDEPENDENT;
+
+  ASSERT_TRUE(CheckTcpConnectivity(private_addrs_[0], private_addrs_[1]));
+}
+
+TEST_F(TestNrSocketTest, PrivateToPublicConnectivityTcp)
+{
+  RefPtr<TestNat> nat(CreatePrivateAddrs(1, "127.0.0.1", IPPROTO_TCP));
+  nat->filtering_type_ = TestNat::ENDPOINT_INDEPENDENT;
+  nat->mapping_type_ = TestNat::ENDPOINT_INDEPENDENT;
+  CreatePublicAddrs(1, "127.0.0.1", IPPROTO_TCP);
+
+  ASSERT_TRUE(CheckTcpConnectivity(private_addrs_[0], public_addrs_[0]));
+}
+
+TEST_F(TestNrSocketTest, NoConnectivityBetweenSubnetsTcp)
+{
+  RefPtr<TestNat> nat1(CreatePrivateAddrs(1, "127.0.0.1", IPPROTO_TCP));
+  nat1->filtering_type_ = TestNat::ENDPOINT_INDEPENDENT;
+  nat1->mapping_type_ = TestNat::ENDPOINT_INDEPENDENT;
+  RefPtr<TestNat> nat2(CreatePrivateAddrs(1, "127.0.0.1", IPPROTO_TCP));
+  nat2->filtering_type_ = TestNat::ENDPOINT_INDEPENDENT;
+  nat2->mapping_type_ = TestNat::ENDPOINT_INDEPENDENT;
+
+  ASSERT_FALSE(CheckTcpConnectivity(private_addrs_[0], private_addrs_[1]));
+}
+
+TEST_F(TestNrSocketTest, NoConnectivityPublicToPrivateTcp)
+{
+  RefPtr<TestNat> nat(CreatePrivateAddrs(1, "127.0.0.1", IPPROTO_TCP));
+  nat->filtering_type_ = TestNat::ENDPOINT_INDEPENDENT;
+  nat->mapping_type_ = TestNat::ENDPOINT_INDEPENDENT;
+  CreatePublicAddrs(1, "127.0.0.1", IPPROTO_TCP);
+
+  ASSERT_FALSE(CheckTcpConnectivity(public_addrs_[0], private_addrs_[0]));
+}
 
 int main(int argc, char **argv)
 {

media/mtransport/test_nr_socket.cpp

@@ -192,7 +192,7 @@ TestNrSocket::~TestNrSocket() {
   nat_->erase_socket(this);
 }
 
-RefPtr<NrSocket> TestNrSocket::create_external_socket(
+RefPtr<NrSocketBase> TestNrSocket::create_external_socket(
     const nr_transport_addr &dest_addr) const {
   MOZ_ASSERT(nat_->enabled_);
   MOZ_ASSERT(!nat_->is_an_internal_tuple(dest_addr));
@@ -202,8 +202,9 @@ RefPtr<NrSocket> TestNrSocket::create_external_socket(
 
   // Open the socket on an arbitrary port, on the same address.
   // TODO(bug 1170299): Remove const_cast when no longer necessary
-  if ((r = nr_transport_addr_copy(&nat_external_addr,
+  if ((r = nr_transport_addr_copy(
-                                  const_cast<nr_transport_addr*>(&my_addr_)))) {
+             &nat_external_addr,
+             const_cast<nr_transport_addr*>(&internal_socket_->my_addr())))) {
     r_log(LOG_GENERIC,LOG_CRIT, "%s: Failure in nr_transport_addr_copy: %d",
                                 __FUNCTION__, r);
     return nullptr;
@@ -215,9 +216,10 @@ RefPtr<NrSocket> TestNrSocket::create_external_socket(
     return nullptr;
   }
 
-  RefPtr<NrSocket> external_socket = new NrSocket;
+  RefPtr<NrSocketBase> external_socket;
+  r = NrSocketBase::CreateSocket(&nat_external_addr, &external_socket);
 
-  if ((r = external_socket->create(&nat_external_addr))) {
+  if (r) {
     r_log(LOG_GENERIC,LOG_CRIT, "%s: Failure in NrSocket::create: %d",
                                 __FUNCTION__, r);
     return nullptr;
@@ -226,13 +228,49 @@ RefPtr<NrSocket> TestNrSocket::create_external_socket(
   return external_socket;
 }
 
+int TestNrSocket::create(nr_transport_addr *addr) {
+  return NrSocketBase::CreateSocket(addr, &internal_socket_);
+}
+
+int TestNrSocket::getaddr(nr_transport_addr *addrp) {
+  return internal_socket_->getaddr(addrp);
+}
+
+void TestNrSocket::close() {
+  // TODO: close port mappings too?
+  internal_socket_->close();
+}
+
+int TestNrSocket::listen(int backlog) {
+  MOZ_ASSERT(internal_socket_->my_addr().protocol == IPPROTO_TCP);
+  r_log(LOG_GENERIC, LOG_DEBUG,
+        "TestNrSocket %s listening",
+        internal_socket_->my_addr().as_string);
+
+  return internal_socket_->listen(backlog);
+}
+
+int TestNrSocket::accept(nr_transport_addr *addrp, nr_socket **sockp) {
+  MOZ_ASSERT(internal_socket_->my_addr().protocol == IPPROTO_TCP);
+  int r = internal_socket_->accept(addrp, sockp);
+  if (r) {
+    return r;
+  }
+
+  if (nat_->enabled_ && !nat_->is_an_internal_tuple(*addrp)) {
+    nr_socket_destroy(sockp);
+    return R_IO_ERROR;
+  }
+
+  return 0;
+}
+
 int TestNrSocket::sendto(const void *msg, size_t len,
                          int flags, nr_transport_addr *to) {
-  MOZ_ASSERT(my_addr_.protocol != IPPROTO_TCP);
+  MOZ_ASSERT(internal_socket_->my_addr().protocol != IPPROTO_TCP);
-  ASSERT_ON_THREAD(ststhread_);
 
   if (!nat_->enabled_ || nat_->is_an_internal_tuple(*to)) {
-    return NrSocket::sendto(msg, len, flags, to);
+    return internal_socket_->sendto(msg, len, flags, to);
   }
 
   destroy_stale_port_mappings();
@@ -251,7 +289,7 @@ int TestNrSocket::sendto(const void *msg, size_t len,
     // See if we have already made the external socket we need to use.
     PortMapping *similar_port_mapping =
       get_port_mapping(*to, nat_->mapping_type_);
-    RefPtr<NrSocket> external_socket;
+    RefPtr<NrSocketBase> external_socket;
 
     if (similar_port_mapping) {
       external_socket = similar_port_mapping->external_socket_;
@@ -270,7 +308,7 @@ int TestNrSocket::sendto(const void *msg, size_t len,
       // Make sure the new port mapping is ready to receive traffic if the
       // TestNrSocket is already waiting.
       port_mapping->async_wait(NR_ASYNC_WAIT_READ,
-                              port_mapping_readable_callback,
+                              socket_readable_callback,
                               this,
                               (char*)__FUNCTION__,
                               __LINE__);
@@ -285,8 +323,7 @@ int TestNrSocket::sendto(const void *msg, size_t len,
 int TestNrSocket::recvfrom(void *buf, size_t maxlen,
                            size_t *len, int flags,
                            nr_transport_addr *from) {
-  MOZ_ASSERT(my_addr_.protocol != IPPROTO_TCP);
+  MOZ_ASSERT(internal_socket_->my_addr().protocol != IPPROTO_TCP);
-  ASSERT_ON_THREAD(ststhread_);
 
   int r;
   bool ingress_allowed = false;
@@ -306,7 +343,7 @@ int TestNrSocket::recvfrom(void *buf, size_t maxlen,
     // If no external socket has data, see if there's any data that was sent
     // directly to the TestNrSocket, and eat it if it isn't supposed to get
     // through.
-    r = NrSocket::recvfrom(buf, maxlen, len, flags, from);
+    r = internal_socket_->recvfrom(buf, maxlen, len, flags, from);
     if (!r) {
       // We do not use allow_ingress() here because that only handles traffic
       // landing on an external port.
@@ -315,7 +352,7 @@ int TestNrSocket::recvfrom(void *buf, size_t maxlen,
       if (!ingress_allowed) {
         r_log(LOG_GENERIC, LOG_INFO, "TestNrSocket %s denying ingress from %s: "
                                      "Not behind the same NAT",
-                                     my_addr_.as_string,
+                                     internal_socket_->my_addr().as_string,
                                      from->as_string);
       }
     }
@@ -348,7 +385,7 @@ bool TestNrSocket::allow_ingress(const nr_transport_addr &from,
   if (!(*port_mapping_used)) {
     r_log(LOG_GENERIC, LOG_INFO, "TestNrSocket %s denying ingress from %s: "
                                  "Filtered",
-                                 my_addr_.as_string,
+                                 internal_socket_->my_addr().as_string,
                                  from.as_string);
     return false;
   }
@@ -356,7 +393,7 @@ bool TestNrSocket::allow_ingress(const nr_transport_addr &from,
   if (is_port_mapping_stale(**port_mapping_used)) {
     r_log(LOG_GENERIC, LOG_INFO, "TestNrSocket %s denying ingress from %s: "
                                  "Stale port mapping",
-                                 my_addr_.as_string,
+                                 internal_socket_->my_addr().as_string,
                                  from.as_string);
     return false;
   }
@@ -364,7 +401,7 @@ bool TestNrSocket::allow_ingress(const nr_transport_addr &from,
   if (!nat_->allow_hairpinning_ && nat_->is_my_external_tuple(from)) {
     r_log(LOG_GENERIC, LOG_INFO, "TestNrSocket %s denying ingress from %s: "
                                  "Hairpinning disallowed",
-                                 my_addr_.as_string,
+                                 internal_socket_->my_addr().as_string,
                                  from.as_string);
     return false;
   }
@@ -373,7 +410,6 @@ bool TestNrSocket::allow_ingress(const nr_transport_addr &from,
 }
 
 int TestNrSocket::connect(nr_transport_addr *addr) {
-  ASSERT_ON_THREAD(ststhread_);
 
   if (connect_invoked_ || !port_mappings_.empty()) {
     MOZ_CRASH("TestNrSocket::connect() called more than once!");
@@ -386,17 +422,21 @@ int TestNrSocket::connect(nr_transport_addr *addr) {
                                       // we don't normally connect on UDP.
       || nat_->is_an_internal_tuple(*addr)) {
     // This will set connect_invoked_
-    return NrSocket::connect(addr);
+    return internal_socket_->connect(addr);
   }
 
-  RefPtr<NrSocket> external_socket(create_external_socket(*addr));
+  RefPtr<NrSocketBase> external_socket(create_external_socket(*addr));
   if (!external_socket) {
     return R_INTERNAL;
   }
 
   PortMapping *port_mapping = create_port_mapping(*addr, external_socket);
   port_mappings_.push_back(port_mapping);
-  port_mapping->external_socket_->connect(addr);
+  int r = port_mapping->external_socket_->connect(addr);
+  if (r && r != R_WOULDBLOCK) {
+    return r;
+  }
+
   port_mapping->last_used_ = PR_IntervalNow();
 
   if (poll_flags() & PR_POLL_READ) {
@@ -407,19 +447,22 @@ int TestNrSocket::connect(nr_transport_addr *addr) {
                              __LINE__);
   }
 
-  return 0;
+  return r;
 }
 
 int TestNrSocket::write(const void *msg, size_t len, size_t *written) {
-  ASSERT_ON_THREAD(ststhread_);
 
   if (port_mappings_.empty()) {
     // The no-nat case, just pass call through.
     r_log(LOG_GENERIC, LOG_INFO, "TestNrSocket %s writing",
           my_addr().as_string);
 
-    return NrSocket::write(msg, len, written);
+    return internal_socket_->write(msg, len, written);
   } else {
+    destroy_stale_port_mappings();
+    if (port_mappings_.empty()) {
+      return -1;
+    }
     // This is TCP only
     MOZ_ASSERT(port_mappings_.size() == 1);
     r_log(LOG_GENERIC, LOG_INFO,
@@ -432,10 +475,9 @@ int TestNrSocket::write(const void *msg, size_t len, size_t *written) {
 }
 
 int TestNrSocket::read(void *buf, size_t maxlen, size_t *len) {
-  ASSERT_ON_THREAD(ststhread_);
 
   if (port_mappings_.empty()) {
-    return NrSocket::read(buf, maxlen, len);
+    return internal_socket_->read(buf, maxlen, len);
   } else {
     MOZ_ASSERT(port_mappings_.size() == 1);
     return port_mappings_.front()->external_socket_->read(buf, maxlen, len);
@@ -444,25 +486,41 @@ int TestNrSocket::read(void *buf, size_t maxlen, size_t *len) {
 
 int TestNrSocket::async_wait(int how, NR_async_cb cb, void *cb_arg,
                              char *function, int line) {
-  ASSERT_ON_THREAD(ststhread_);
+  r_log(LOG_GENERIC, LOG_DEBUG, "TestNrSocket %s waiting for %s",
+                                internal_socket_->my_addr().as_string,
+                                how == NR_ASYNC_WAIT_READ ? "read" : "write");
 
-  // Make sure we're waiting on the socket for the internal address
+  int r;
-  int r = NrSocket::async_wait(how, cb, cb_arg, function, line);
+
+  if (how == NR_ASYNC_WAIT_READ) {
+    NrSocketBase::async_wait(how, cb, cb_arg, function, line);
+
+    // Make sure we're waiting on the socket for the internal address
+    r = internal_socket_->async_wait(how,
+                                     socket_readable_callback,
+                                     this,
+                                     function,
+                                     line);
+  } else {
+    // For write, just use the readiness of the internal socket, since we queue
+    // everything for the port mappings.
+    r = internal_socket_->async_wait(how,
+                                     cb,
+                                     cb_arg,
+                                     function,
+                                     line);
+  }
 
   if (r) {
     return r;
   }
 
-  r_log(LOG_GENERIC, LOG_DEBUG, "TestNrSocket %s waiting for %s",
-                                my_addr_.as_string,
-                                how == NR_ASYNC_WAIT_READ ? "read" : "write");
-
   if (is_tcp_connection_behind_nat()) {
     // Bypass all port-mapping related logic
     return 0;
   }
 
-  if (my_addr_.protocol == IPPROTO_TCP) {
+  if (internal_socket_->my_addr().protocol == IPPROTO_TCP) {
     // For a TCP connection through a simulated NAT, these signals are
     // just passed through.
     MOZ_ASSERT(port_mappings_.size() == 1);
@@ -478,7 +536,7 @@ int TestNrSocket::async_wait(int how, NR_async_cb cb, void *cb_arg,
     for (PortMapping *port_mapping : port_mappings_) {
       // Be ready to receive traffic on our port mappings
       r = port_mapping->async_wait(how,
-                                   port_mapping_readable_callback,
+                                   socket_readable_callback,
                                    this,
                                    function,
                                    line);
@@ -498,18 +556,18 @@ void TestNrSocket::cancel_port_mapping_async_wait(int how) {
 }
 
 int TestNrSocket::cancel(int how) {
-  ASSERT_ON_THREAD(ststhread_);
 
   r_log(LOG_GENERIC, LOG_DEBUG, "TestNrSocket %s stop waiting for %s",
-        my_addr_.as_string,
+        internal_socket_->my_addr().as_string,
         how == NR_ASYNC_WAIT_READ ? "read" : "write");
 
   // Writable callbacks are decoupled except for the TCP case
-  if (how == NR_ASYNC_WAIT_READ || my_addr_.protocol == IPPROTO_TCP) {
+  if (how == NR_ASYNC_WAIT_READ ||
+      internal_socket_->my_addr().protocol == IPPROTO_TCP) {
     cancel_port_mapping_async_wait(how);
   }
 
-  return NrSocket::cancel(how);
+  return internal_socket_->cancel(how);
 }
 
 bool TestNrSocket::has_port_mappings() const {
@@ -548,7 +606,7 @@ void TestNrSocket::destroy_stale_port_mappings() {
     if (is_port_mapping_stale(**temp)) {
       r_log(LOG_GENERIC, LOG_INFO,
             "TestNrSocket %s destroying port mapping %s -> %s",
-            my_addr_.as_string,
+            internal_socket_->my_addr().as_string,
             (*temp)->external_socket_->my_addr().as_string,
             (*temp)->remote_address_.as_string);
 
@@ -557,34 +615,31 @@ void TestNrSocket::destroy_stale_port_mappings() {
   }
 }
 
-void TestNrSocket::port_mapping_readable_callback(void *ext_sock_v,
+void TestNrSocket::socket_readable_callback(void *real_sock_v,
-                                                  int how,
+                                             int how,
-                                                  void *test_sock_v) {
+                                             void *test_sock_v) {
   TestNrSocket *test_socket = static_cast<TestNrSocket*>(test_sock_v);
-  NrSocket *external_socket = static_cast<NrSocket*>(ext_sock_v);
+  NrSocket *real_socket = static_cast<NrSocket*>(real_sock_v);
 
-  test_socket->on_port_mapping_readable(external_socket);
+  test_socket->on_socket_readable(real_socket);
 }
 
-void TestNrSocket::on_port_mapping_readable(NrSocket *external_socket) {
+void TestNrSocket::on_socket_readable(NrSocketBase *real_socket) {
-  if (!readable_socket_) {
+  if (!readable_socket_ && (real_socket != internal_socket_)) {
-    readable_socket_ = external_socket;
+    readable_socket_ = real_socket;
   }
 
-  // None of our port mappings should be waiting for readable callbacks
-  // if nobody is waiting for readable callbacks from us.
-  MOZ_ASSERT(poll_flags() & PR_POLL_READ);
-
   fire_readable_callback();
 }
 
 void TestNrSocket::fire_readable_callback() {
   MOZ_ASSERT(poll_flags() & PR_POLL_READ);
-  // Stop listening on all mapped sockets; we will start listening again
+  // Stop listening on all real sockets; we will start listening again
   // if the app starts listening to us again.
   cancel_port_mapping_async_wait(NR_ASYNC_WAIT_READ);
+  internal_socket_->cancel(NR_ASYNC_WAIT_READ);
   r_log(LOG_GENERIC, LOG_DEBUG, "TestNrSocket %s ready for read",
-        my_addr_.as_string);
+        internal_socket_->my_addr().as_string);
   fire_callback(NR_ASYNC_WAIT_READ);
 }
 
@@ -592,13 +647,13 @@ void TestNrSocket::port_mapping_writeable_callback(void *ext_sock_v,
                                                    int how,
                                                    void *test_sock_v) {
   TestNrSocket *test_socket = static_cast<TestNrSocket*>(test_sock_v);
-  NrSocket *external_socket = static_cast<NrSocket*>(ext_sock_v);
+  NrSocketBase *external_socket = static_cast<NrSocketBase*>(ext_sock_v);
 
   test_socket->write_to_port_mapping(external_socket);
 }
 
-void TestNrSocket::write_to_port_mapping(NrSocket *external_socket) {
+void TestNrSocket::write_to_port_mapping(NrSocketBase *external_socket) {
-  MOZ_ASSERT(my_addr_.protocol != IPPROTO_TCP);
+  MOZ_ASSERT(internal_socket_->my_addr().protocol != IPPROTO_TCP);
 
   int r = 0;
   for (PortMapping *port_mapping : port_mappings_) {
@@ -626,15 +681,16 @@ void TestNrSocket::port_mapping_tcp_passthrough_callback(void *ext_sock_v,
   TestNrSocket *test_socket = static_cast<TestNrSocket*>(test_sock_v);
   r_log(LOG_GENERIC, LOG_INFO,
         "TestNrSocket %s firing %s callback",
-        test_socket->my_addr().as_string,
+        test_socket->internal_socket_->my_addr().as_string,
         how == NR_ASYNC_WAIT_READ ? "readable" : "writeable");
 
 
-  test_socket->fire_callback(how);
+  test_socket->internal_socket_->fire_callback(how);
 }
 
 bool TestNrSocket::is_tcp_connection_behind_nat() const {
-  return my_addr_.protocol == IPPROTO_TCP && port_mappings_.empty();
+  return internal_socket_->my_addr().protocol == IPPROTO_TCP &&
+         port_mappings_.empty();
 }
 
 TestNrSocket::PortMapping* TestNrSocket::get_port_mapping(
@@ -665,9 +721,9 @@ TestNrSocket::PortMapping* TestNrSocket::get_port_mapping(
 
 TestNrSocket::PortMapping* TestNrSocket::create_port_mapping(
     const nr_transport_addr &remote_address,
-    const RefPtr<NrSocket> &external_socket) const {
+    const RefPtr<NrSocketBase> &external_socket) const {
   r_log(LOG_GENERIC, LOG_INFO, "TestNrSocket %s creating port mapping %s -> %s",
-        my_addr_.as_string,
+        internal_socket_->my_addr().as_string,
         external_socket->my_addr().as_string,
         remote_address.as_string);
 
@@ -676,7 +732,7 @@ TestNrSocket::PortMapping* TestNrSocket::create_port_mapping(
 
 TestNrSocket::PortMapping::PortMapping(
     const nr_transport_addr &remote_address,
-    const RefPtr<NrSocket> &external_socket) :
+    const RefPtr<NrSocketBase> &external_socket) :
   external_socket_(external_socket) {
   // TODO(bug 1170299): Remove const_cast when no longer necessary
   nr_transport_addr_copy(&remote_address_,

media/mtransport/test_nr_socket.h

@@ -167,34 +167,45 @@ class TestNat {
 };
 
 /**
- * Subclass of NrSocket that can simulate things like being behind a NAT, packet
+ * Subclass of NrSocketBase that can simulate things like being behind a NAT,
- * loss, latency, packet rewriting, etc. Also exposes some stuff that assists in
+ * packet loss, latency, packet rewriting, etc. Also exposes some stuff that
- * diagnostics.
+ * assists in diagnostics.
+ * This is accomplished by wrapping an "internal" socket (that handles traffic
+ * behind the NAT), and a collection of "external" sockets (that handle traffic
+ * into/out of the NAT)
  */
-class TestNrSocket : public NrSocket {
+class TestNrSocket : public NrSocketBase {
   public:
     explicit TestNrSocket(TestNat *nat);
 
-    virtual ~TestNrSocket();
-
     bool has_port_mappings() const;
     bool is_my_external_tuple(const nr_transport_addr &addr) const;
 
-    // Overrides of NrSocket
+    // Overrides of NrSocketBase
+    int create(nr_transport_addr *addr) override;
     int sendto(const void *msg, size_t len,
                int flags, nr_transport_addr *to) override;
     int recvfrom(void * buf, size_t maxlen,
                  size_t *len, int flags,
                  nr_transport_addr *from) override;
+    int getaddr(nr_transport_addr *addrp) override;
+    void close() override;
     int connect(nr_transport_addr *addr) override;
     int write(const void *msg, size_t len, size_t *written) override;
     int read(void *buf, size_t maxlen, size_t *len) override;
 
+    int listen(int backlog) override;
+    int accept(nr_transport_addr *addrp, nr_socket **sockp) override;
     int async_wait(int how, NR_async_cb cb, void *cb_arg,
                    char *function, int line) override;
     int cancel(int how) override;
 
+    // Need override since this is virtual in NrSocketBase
+    NS_INLINE_DECL_THREADSAFE_REFCOUNTING(TestNrSocket, override)
+
   private:
+    virtual ~TestNrSocket();
+
     class UdpPacket {
       public:
         UdpPacket(const void *msg, size_t len, const nr_transport_addr &addr) :
@@ -215,7 +226,7 @@ class TestNrSocket : public NrSocket {
     class PortMapping {
       public:
         PortMapping(const nr_transport_addr &remote_address,
-                    const RefPtr<NrSocket> &external_socket);
+                    const RefPtr<NrSocketBase> &external_socket);
 
         int sendto(const void *msg, size_t len, const nr_transport_addr &to);
         int async_wait(int how, NR_async_cb cb, void *cb_arg,
@@ -225,7 +236,7 @@ class TestNrSocket : public NrSocket {
         NS_INLINE_DECL_THREADSAFE_REFCOUNTING(PortMapping);
 
         PRIntervalTime last_used_;
-        RefPtr<NrSocket> external_socket_;
+        RefPtr<NrSocketBase> external_socket_;
         // For non-symmetric, most of the data here doesn't matter
         nr_transport_addr remote_address_;
 
@@ -243,10 +254,10 @@ class TestNrSocket : public NrSocket {
                        PortMapping **port_mapping_used) const;
     void destroy_stale_port_mappings();
 
-    static void port_mapping_readable_callback(void *ext_sock_v,
+    static void socket_readable_callback(void *real_sock_v,
-                                               int how,
+                                         int how,
-                                               void *test_sock_v);
+                                         void *test_sock_v);
-    void on_port_mapping_readable(NrSocket *external_socket);
+    void on_socket_readable(NrSocketBase *external_or_internal_socket);
     void fire_readable_callback();
 
     static void port_mapping_tcp_passthrough_callback(void *ext_sock_v,
@@ -257,18 +268,21 @@ class TestNrSocket : public NrSocket {
     static void port_mapping_writeable_callback(void *ext_sock_v,
                                                 int how,
                                                 void *test_sock_v);
-    void write_to_port_mapping(NrSocket *external_socket);
+    void write_to_port_mapping(NrSocketBase *external_socket);
     bool is_tcp_connection_behind_nat() const;
 
     PortMapping* get_port_mapping(const nr_transport_addr &remote_addr,
                                   TestNat::NatBehavior filter) const;
     PortMapping* create_port_mapping(
         const nr_transport_addr &remote_addr,
-        const RefPtr<NrSocket> &external_socket) const;
+        const RefPtr<NrSocketBase> &external_socket) const;
-    RefPtr<NrSocket> create_external_socket(
+    RefPtr<NrSocketBase> create_external_socket(
         const nr_transport_addr &remote_addr) const;
 
-    RefPtr<NrSocket> readable_socket_;
+    RefPtr<NrSocketBase> readable_socket_;
+    // The socket for the "internal" address; used to talk to stuff behind the
+    // same nat.
+    RefPtr<NrSocketBase> internal_socket_;
     RefPtr<TestNat> nat_;
     // Since our comparison logic is different depending on what kind of NAT
     // we simulate, and the STL does not make it very easy to switch out the

media/mtransport/third_party/nICEr/src/ice/ice_socket.c

@@ -67,8 +67,7 @@ static void nr_ice_socket_readable_cb(NR_SOCKET s, int how, void *cb_arg)
     if(r=nr_socket_recvfrom(sock->sock,buf,sizeof(buf),&len_s,0,&addr)){
       if (r != R_WOULDBLOCK && (sock->type != NR_ICE_SOCKET_TYPE_DGRAM)) {
         /* Report this error upward. Bug 946423 */
-        r_log(LOG_ICE,LOG_ERR,"ICE(%s): Error %d on reliable socket. Abandoning.",sock->ctx->label, r);
+        r_log(LOG_ICE,LOG_ERR,"ICE(%s): Error %d on reliable socket(%p). Abandoning.",sock->ctx->label, r, s);
-        NR_ASYNC_CANCEL(s, NR_ASYNC_WAIT_READ);
       }
       return;
     }

media/mtransport/third_party/nICEr/src/stun/nr_socket_buffered_stun.c

@@ -250,7 +250,15 @@ abort:
 
 static void nr_socket_buffered_stun_failed(nr_socket_buffered_stun *sock)
   {
+    NR_SOCKET fd;
+
     sock->read_state = NR_ICE_SOCKET_READ_FAILED;
+
+    /* Cancel waiting on the socket */
+    if (sock->inner && !nr_socket_getfd(sock->inner, &fd)) {
+      NR_ASYNC_CANCEL(fd, NR_ASYNC_WAIT_WRITE);
+      NR_ASYNC_CANCEL(fd, NR_ASYNC_WAIT_READ);
+    }
   }
 
 static int nr_socket_buffered_stun_recvfrom(void *obj,void * restrict buf,