[trafficserver] branch master updated: fix the missing lock in TSVConnFdCreate api

[sc...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

scw00 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/trafficserver.git


The following commit(s) were added to refs/heads/master by this push:
     new 8551244  fix the missing lock in TSVConnFdCreate api
8551244 is described below

commit 85512448f3faf3ede5ba8b63f774647c0701142f
Author: scw00 <sc...@apache.org>
AuthorDate: Tue Oct 10 15:51:59 2017 +0800

    fix the missing lock in TSVConnFdCreate api
---
 iocore/net/P_UnixNet.h           |  9 ++++-----
 iocore/net/UnixNetAccept.cc      |  6 ++----
 iocore/net/UnixNetVConnection.cc | 13 ++++++++-----
 proxy/InkAPI.cc                  | 11 ++++++++---
 4 files changed, 22 insertions(+), 17 deletions(-)

diff --git a/iocore/net/P_UnixNet.h b/iocore/net/P_UnixNet.h
index ce76c45..e9c6948 100644
--- a/iocore/net/P_UnixNet.h
+++ b/iocore/net/P_UnixNet.h
@@ -335,8 +335,7 @@ check_throttle_warning()
 // of emergency throttle).
 //
 // Hyper Emergency throttle when we are very close to exhausting file
-// descriptors.  Close the connection immediately, the upper levels
-// will recover.
+// descriptors.
 //
 TS_INLINE bool
 check_emergency_throttle(Connection &con)
@@ -348,9 +347,9 @@ check_emergency_throttle(Connection &con)
     emergency_throttle_time = Thread::get_hrtime() + (over * over) * HRTIME_SECOND;
     RecSignalWarning(REC_SIGNAL_SYSTEM_ERROR, "too many open file descriptors, emergency throttling");
     int hyper_emergency = fds_limit - HYPER_EMERGENCY_THROTTLE;
-    if (fd > hyper_emergency)
-      con.close();
-    return true;
+    if (fd > hyper_emergency) {
+      return true;
+    }
   }
   return false;
 }
diff --git a/iocore/net/UnixNetAccept.cc b/iocore/net/UnixNetAccept.cc
index 80711c7..c6d9e95 100644
--- a/iocore/net/UnixNetAccept.cc
+++ b/iocore/net/UnixNetAccept.cc
@@ -271,10 +271,8 @@ NetAccept::do_blocking_accept(EThread *t)
 
     // The con.fd may exceed the limitation of check_net_throttle() because we do blocking accept here.
     if (check_emergency_throttle(con)) {
-      // The `con' could be closed if there is hyper emergency
-      if (con.fd == NO_FD) {
-        return 0;
-      }
+      con.close();
+      return 0;
     }
 
     // Use 'nullptr' to Bypass thread allocator
diff --git a/iocore/net/UnixNetVConnection.cc b/iocore/net/UnixNetVConnection.cc
index 0e97ffa..808fb53 100644
--- a/iocore/net/UnixNetVConnection.cc
+++ b/iocore/net/UnixNetVConnection.cc
@@ -1305,14 +1305,14 @@ UnixNetVConnection::connectUp(EThread *t, int fd)
 
   if (check_emergency_throttle(con)) {
     // The `con' could be closed if there is hyper emergency
-    if (con.fd == NO_FD) {
+    if (fd == NO_FD) {
       // We need to decrement the stat because close_UnixNetVConnection only decrements with a valid connection descriptor.
       NET_SUM_GLOBAL_DYN_STAT(net_connections_currently_open_stat, -1);
-      // Set errno force to EMFILE (reached limit for open file descriptors)
-      errno = EMFILE;
-      res   = -errno;
-      goto fail;
     }
+    // Set errno force to EMFILE (reached limit for open file descriptors)
+    errno = EMFILE;
+    res   = -errno;
+    goto fail;
   }
 
   // Must connect after EventIO::Start() to avoid a race condition
@@ -1344,6 +1344,9 @@ UnixNetVConnection::connectUp(EThread *t, int fd)
 fail:
   lerrno = -res;
   action_.continuation->handleEvent(NET_EVENT_OPEN_FAILED, (void *)(intptr_t)res);
+  if (fd != NO_FD) {
+    con.fd = NO_FD;
+  }
   free(t);
   return CONNECT_FAILURE;
 }
diff --git a/proxy/InkAPI.cc b/proxy/InkAPI.cc
index a338c72..3c164e0 100644
--- a/proxy/InkAPI.cc
+++ b/proxy/InkAPI.cc
@@ -6478,12 +6478,13 @@ TSVConn
 TSVConnFdCreate(int fd)
 {
   UnixNetVConnection *vc;
+  EThread *t = this_ethread();
 
   if (unlikely(fd == NO_FD)) {
     return nullptr;
   }
 
-  vc = (UnixNetVConnection *)netProcessor.allocate_vc(this_ethread());
+  vc = (UnixNetVConnection *)netProcessor.allocate_vc(t);
   if (vc == nullptr) {
     return nullptr;
   }
@@ -6496,11 +6497,15 @@ TSVConnFdCreate(int fd)
 
   vc->id          = net_next_connection_number();
   vc->submit_time = Thread::get_hrtime();
+  vc->mutex       = new_ProxyMutex();
   vc->set_is_transparent(false);
-  vc->mutex = new_ProxyMutex();
   vc->set_context(NET_VCONNECTION_OUT);
 
-  if (vc->connectUp(this_ethread(), fd) != CONNECT_SUCCESS) {
+  // We should take the nh's lock and vc's lock before we get into the connectUp
+  SCOPED_MUTEX_LOCK(lock, get_NetHandler(t)->mutex, t);
+  SCOPED_MUTEX_LOCK(lock2, vc->mutex, t);
+
+  if (vc->connectUp(t, fd) != CONNECT_SUCCESS) {
     return nullptr;
   }
 

-- 
To stop receiving notification emails like this one, please contact
['"commits@trafficserver.apache.org" <co...@trafficserver.apache.org>'].