You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by zw...@apache.org on 2010/10/23 00:43:20 UTC

svn commit: r1026519 - in /trafficserver/traffic/trunk: ./ iocore/net/ libinktomi++/ proxy/http2/

Author: zwoop
Date: Fri Oct 22 22:43:20 2010
New Revision: 1026519

URL: http://svn.apache.org/viewvc?rev=1026519&view=rev
Log:
TS-496 Don't "leak" VCs when using accept thread

Modified:
    trafficserver/traffic/trunk/CHANGES
    trafficserver/traffic/trunk/iocore/net/P_UnixNetProcessor.h
    trafficserver/traffic/trunk/iocore/net/P_UnixNetVConnection.h
    trafficserver/traffic/trunk/iocore/net/UnixNetAccept.cc
    trafficserver/traffic/trunk/iocore/net/UnixNetProcessor.cc
    trafficserver/traffic/trunk/iocore/net/UnixNetVConnection.cc
    trafficserver/traffic/trunk/libinktomi++/ink_queue.h
    trafficserver/traffic/trunk/proxy/http2/HttpAccept.h
    trafficserver/traffic/trunk/proxy/http2/HttpProxyServerMain.cc

Modified: trafficserver/traffic/trunk/CHANGES
URL: http://svn.apache.org/viewvc/trafficserver/traffic/trunk/CHANGES?rev=1026519&r1=1026518&r2=1026519&view=diff
==============================================================================
--- trafficserver/traffic/trunk/CHANGES (original)
+++ trafficserver/traffic/trunk/CHANGES Fri Oct 22 22:43:20 2010
@@ -2,6 +2,28 @@
 
 Changes with Apache Traffic Server 2.1.4
 
+  *) Accept threads can leak some amount of memory. This patch also
+   supports multiple accept threads (very experimental!) [TS-496].
+
+  *) HttpSM has an assertion that checks the client URL against the
+   cache URL, which breaks INKSetCacheUrl [TS-495].
+
+  *) Return value from pcre_exec tested incorrectly [TS-493].
+
+  *) Improved loop detection using the Via header [TS-490].
+
+  *) Fixes for Solaris build (yay, it builds!).
+
+  *) Remove filter.config remnants [TS-486].
+
+  *) Cleanup in InkAPI [TS-485].
+
+  *) Move PKGSYSUSER to ink_config.h.in [TS-482].
+
+  *) Unresponsive server can stall ATS [TS-480].
+
+  *) UrlRewrite cleanup [TS-434].
+
   *) Build TS with clang (author: Igor Galic) [TS-427].
 
   *) Better support and handling of DNS round-robin options (author: Zhao

Modified: trafficserver/traffic/trunk/iocore/net/P_UnixNetProcessor.h
URL: http://svn.apache.org/viewvc/trafficserver/traffic/trunk/iocore/net/P_UnixNetProcessor.h?rev=1026519&r1=1026518&r2=1026519&view=diff
==============================================================================
--- trafficserver/traffic/trunk/iocore/net/P_UnixNetProcessor.h (original)
+++ trafficserver/traffic/trunk/iocore/net/P_UnixNetProcessor.h Fri Oct 22 22:43:20 2010
@@ -75,7 +75,6 @@ public:
 
   char *throttle_error_message;
   Event *accept_thread_event;
-  AtomicSLL<NetAccept, NetAccept::Link_link> accepts_on_thread;
 
   // offsets for per thread data structures
   off_t netHandler_offset;

Modified: trafficserver/traffic/trunk/iocore/net/P_UnixNetVConnection.h
URL: http://svn.apache.org/viewvc/trafficserver/traffic/trunk/iocore/net/P_UnixNetVConnection.h?rev=1026519&r1=1026518&r2=1026519&view=diff
==============================================================================
--- trafficserver/traffic/trunk/iocore/net/P_UnixNetVConnection.h (original)
+++ trafficserver/traffic/trunk/iocore/net/P_UnixNetVConnection.h Fri Oct 22 22:43:20 2010
@@ -234,6 +234,7 @@ public:
   int recursion;
   ink_hrtime submit_time;
   OOB_callback *oob_ptr;
+  bool from_accept_thread;
 
   int startEvent(int event, Event *e);
   int acceptEvent(int event, Event *e);

Modified: trafficserver/traffic/trunk/iocore/net/UnixNetAccept.cc
URL: http://svn.apache.org/viewvc/trafficserver/traffic/trunk/iocore/net/UnixNetAccept.cc?rev=1026519&r1=1026518&r2=1026519&view=diff
==============================================================================
--- trafficserver/traffic/trunk/iocore/net/UnixNetAccept.cc (original)
+++ trafficserver/traffic/trunk/iocore/net/UnixNetAccept.cc Fri Oct 22 22:43:20 2010
@@ -167,18 +167,16 @@ net_accept_main_blocking(NetAccept * na,
 }
 
 
-// Functions all THREAD_FREE and THREAD_ALLOC to be performed
-// for both SSL and regular UnixNetVConnection transparent to
-// accept functions.
 UnixNetVConnection *
 NetAccept::allocateThread(EThread * t)
 {
-  return ((UnixNetVConnection *) THREAD_ALLOC(netVCAllocator, t));
+  return ((UnixNetVConnection *)THREAD_ALLOC(netVCAllocator, t));
 }
 
 void
 NetAccept::freeThread(UnixNetVConnection * vc, EThread * t)
 {
+  ink_assert(!vc->from_accept_thread);
   THREAD_FREE(vc, netVCAllocator, t);
 }
 
@@ -203,8 +201,6 @@ NetAccept::init_accept_loop()
     action_->continuation->mutex = new_ProxyMutex();
     action_->mutex = action_->continuation->mutex;
   }
-  do_listen(BLOCKING);
-  unix_netProcessor.accepts_on_thread.push(this);
   SET_CONTINUATION_HANDLER(this, &NetAccept::acceptLoopEvent);
   eventProcessor.spawn_thread(this);
 }
@@ -247,7 +243,8 @@ NetAccept::init_accept_per_thread()
   else
     SET_HANDLER((NetAcceptHandler) & NetAccept::acceptEvent);
   period = ACCEPT_PERIOD;
-  NetAccept *a = this;
+
+  NetAccept *a;
   n = eventProcessor.n_threads_for_type[ET_NET];
   for (i = 0; i < n; i++) {
     if (i < n - 1) {
@@ -302,8 +299,9 @@ NetAccept::do_blocking_accept(NetAccept 
   //added by YTS Team, yamsat
   do {
     vc = (UnixNetVConnection *) master_na->alloc_cache;
-    if (!vc) {
-      vc = allocateThread(t);
+    if (likely(!vc)) {
+      vc = (UnixNetVConnection *)netVCAllocator.alloc(); // Don't use thread / proxy allocation
+      vc->from_accept_thread = true;
       vc->id = net_next_connection_number();
       master_na->alloc_cache = vc;
 #if TS_USE_DETAILED_LOG

Modified: trafficserver/traffic/trunk/iocore/net/UnixNetProcessor.cc
URL: http://svn.apache.org/viewvc/trafficserver/traffic/trunk/iocore/net/UnixNetProcessor.cc?rev=1026519&r1=1026518&r2=1026519&view=diff
==============================================================================
--- trafficserver/traffic/trunk/iocore/net/UnixNetProcessor.cc (original)
+++ trafficserver/traffic/trunk/iocore/net/UnixNetProcessor.cc Fri Oct 22 22:43:20 2010
@@ -80,9 +80,9 @@ NetProcessor::accept(Continuation * cont
   (void) bound_sockaddr;        // NT only
   (void) bound_sockaddr_size;   // NT only
   NetDebug("iocore_net_processor",
-	   "NetProcessor::accept - port %d,recv_bufsize %d, send_bufsize %d, sockopt 0x%0lX",
-	   port, recv_bufsize, send_bufsize, sockopt_flags
-	   );
+           "NetProcessor::accept - port %d,recv_bufsize %d, send_bufsize %d, sockopt 0x%0lX",
+           port, recv_bufsize, send_bufsize, sockopt_flags
+           );
 
   AcceptOptions opt;
   opt.port = port;
@@ -97,28 +97,28 @@ NetProcessor::accept(Continuation * cont
                                                       frequent_accept,
                                                       net_accept,
                                                       accept_ip,
-						      opt
-						      );
+                                                      opt
+                                                      );
 }
 
 Action *
 NetProcessor::main_accept(Continuation * cont, SOCKET fd,
                           sockaddr * bound_sockaddr, int *bound_sockaddr_size,
                           bool accept_only,
-			  AcceptOptions const& opt
-			  )
+                          AcceptOptions const& opt
+                          )
 {
   (void) accept_only;           // NT only
   NetDebug("iocore_net_processor", "NetProcessor::main_accept - port %d,recv_bufsize %d, send_bufsize %d, sockopt 0x%0lX",
-        opt.port, opt.recv_bufsize, opt.send_bufsize, opt.sockopt_flags);
+           opt.port, opt.recv_bufsize, opt.send_bufsize, opt.sockopt_flags);
   return ((UnixNetProcessor *) this)->accept_internal(cont, fd,
                                                       bound_sockaddr,
                                                       bound_sockaddr_size,
                                                       true,
                                                       net_accept,
                                                       ((UnixNetProcessor *) this)->incoming_ip_to_bind_saddr,
-						      opt
-						      );
+                                                      opt
+                                                      );
 }
 
 
@@ -131,8 +131,8 @@ UnixNetProcessor::accept_internal(Contin
                                   bool frequent_accept,
                                   AcceptFunction fn,
                                   unsigned int accept_ip,
-				  AcceptOptions const& opt
-				  )
+                                  AcceptOptions const& opt
+                                  )
 {
   EventType et = opt.etype; // setEtype requires non-const ref.
   setEtype(et);
@@ -159,10 +159,20 @@ UnixNetProcessor::accept_internal(Contin
   if (na->callback_on_open)
     na->mutex = cont->mutex;
   if (frequent_accept) { // true
-    if (use_accept_thread) // 0
-      na->init_accept_loop();
-    else
+    if (use_accept_thread)  {
+      if (0 == na->do_listen(BLOCKING)) {
+        NetAccept *a;
+        
+        while (--use_accept_thread > 0) {
+          a = NEW(new NetAccept);
+          *a = *na;
+          a->init_accept_loop();
+        }
+        na->init_accept_loop();
+      }
+    } else {
       na->init_accept_per_thread();
+    }
   } else
     na->init_accept();
   if (bound_sockaddr && bound_sockaddr_size)
@@ -301,7 +311,7 @@ struct CheckConnect:public Continuation
       break;
 
       case NET_EVENT_OPEN_FAILED:
-	NetDebug("iocore_net_connect", "connect Net open failed");
+        NetDebug("iocore_net_connect", "connect Net open failed");
       if (!action_.cancelled)
         action_.continuation->handleEvent(NET_EVENT_OPEN_FAILED, (void *) e);
       break;

Modified: trafficserver/traffic/trunk/iocore/net/UnixNetVConnection.cc
URL: http://svn.apache.org/viewvc/trafficserver/traffic/trunk/iocore/net/UnixNetVConnection.cc?rev=1026519&r1=1026518&r2=1026519&view=diff
==============================================================================
--- trafficserver/traffic/trunk/iocore/net/UnixNetVConnection.cc (original)
+++ trafficserver/traffic/trunk/iocore/net/UnixNetVConnection.cc Fri Oct 22 22:43:20 2010
@@ -784,15 +784,16 @@ UnixNetVConnection::reenable_re(VIO *vio
 }
 
 
-UnixNetVConnection::UnixNetVConnection():
-closed(0), inactivity_timeout_in(0), active_timeout_in(0),
+UnixNetVConnection::UnixNetVConnection()
+  : closed(0), inactivity_timeout_in(0), active_timeout_in(0),
 #ifdef INACTIVITY_TIMEOUT
-  inactivity_timeout(NULL),
+    inactivity_timeout(NULL),
 #else
-  next_inactivity_timeout_at(0),
+    next_inactivity_timeout_at(0),
 #endif
-  active_timeout(NULL), nh(NULL),
-  id(0), ip(0), accept_port(0), port(0), flags(0), recursion(0), submit_time(0), oob_ptr(0)
+    active_timeout(NULL), nh(NULL),
+    id(0), ip(0), accept_port(0), port(0), flags(0), recursion(0), submit_time(0), oob_ptr(0),
+    from_accept_thread(false)
 {
   memset(&local_sa, 0, sizeof local_sa);
   SET_HANDLER((NetVConnHandler) & UnixNetVConnection::startEvent);
@@ -1147,5 +1148,10 @@ UnixNetVConnection::free(EThread *t)
   ink_debug_assert(!active_timeout);
   ink_debug_assert(con.fd == NO_FD);
   ink_debug_assert(t == this_ethread());
-  THREAD_FREE(this, netVCAllocator, t);
+
+  if (from_accept_thread) {
+    netVCAllocator.free(this);  
+  } else {
+    THREAD_FREE(this, netVCAllocator, t);
+  }
 }

Modified: trafficserver/traffic/trunk/libinktomi++/ink_queue.h
URL: http://svn.apache.org/viewvc/trafficserver/traffic/trunk/libinktomi%2B%2B/ink_queue.h?rev=1026519&r1=1026518&r2=1026519&view=diff
==============================================================================
--- trafficserver/traffic/trunk/libinktomi++/ink_queue.h (original)
+++ trafficserver/traffic/trunk/libinktomi++/ink_queue.h Fri Oct 22 22:43:20 2010
@@ -165,10 +165,11 @@ extern "C"
   static inline void *ink_freelist_new(InkFreeList * f)
   {
     void *retval = NULL;
-      ink_mutex_acquire(&(f->inkfreelist_mutex));
-      retval = ink_freelist_new_wrap(f);
-      ink_mutex_release(&(f->inkfreelist_mutex));
-      return retval;
+
+    ink_mutex_acquire(&(f->inkfreelist_mutex));
+    retval = ink_freelist_new_wrap(f);
+    ink_mutex_release(&(f->inkfreelist_mutex));
+    return retval;
   }
 
   inkcoreapi void ink_freelist_free_wrap(InkFreeList * f, void *item);

Modified: trafficserver/traffic/trunk/proxy/http2/HttpAccept.h
URL: http://svn.apache.org/viewvc/trafficserver/traffic/trunk/proxy/http2/HttpAccept.h?rev=1026519&r1=1026518&r2=1026519&view=diff
==============================================================================
--- trafficserver/traffic/trunk/proxy/http2/HttpAccept.h (original)
+++ trafficserver/traffic/trunk/proxy/http2/HttpAccept.h Fri Oct 22 22:43:20 2010
@@ -30,11 +30,29 @@
 #include "HTTP.h"
 #include "IPRange.h"
 
-class HttpAccept:public Continuation
+
+/**
+   The continuation mutex is NULL to allow parellel accepts in NT. The
+   only state used by the handler is attr and backdoor which is setup
+   at the beginning of time and never changed. No state is recorded by
+   the handler. So a NULL mutex is safe.
+
+*/
+
+class HttpAccept: public Continuation
 {
 public:
-  HttpAccept(int aattr, bool abackdoor = false);
-   ~HttpAccept();
+ HttpAccept(int aattr, bool abackdoor = false)
+   : Continuation(NULL), backdoor(abackdoor), attr(aattr)
+  {
+    SET_HANDLER(&HttpAccept::mainEvent);
+    return;
+  }
+
+  ~HttpAccept()
+  {
+    return;
+  }
 
   int mainEvent(int event, void *netvc);
 
@@ -45,28 +63,5 @@ private:
     HttpAccept & operator =(const HttpAccept &);
 };
 
-/**
-  The continuation mutex is NULL to allow parellel accepts in NT. The
-  only state used by the handler is attr and backdoor which is setup
-  at the beginning of time and never changed. No state is recorded by
-  the handler. So a NULL mutex is safe.
 
-*/
-inline
-HttpAccept::HttpAccept(int aattr, bool abackdoor)
-  :
-Continuation(NULL),
-backdoor(abackdoor),
-attr(aattr)
-{
-  SET_HANDLER(&HttpAccept::mainEvent);
-  return;
-}
-
-inline
-HttpAccept::~
-HttpAccept()
-{
-  return;
-}
 #endif

Modified: trafficserver/traffic/trunk/proxy/http2/HttpProxyServerMain.cc
URL: http://svn.apache.org/viewvc/trafficserver/traffic/trunk/proxy/http2/HttpProxyServerMain.cc?rev=1026519&r1=1026518&r2=1026519&view=diff
==============================================================================
--- trafficserver/traffic/trunk/proxy/http2/HttpProxyServerMain.cc (original)
+++ trafficserver/traffic/trunk/proxy/http2/HttpProxyServerMain.cc Fri Oct 22 22:43:20 2010
@@ -289,11 +289,11 @@ start_HttpProxyServer(int fd, int port, 
       type = attr.type;
       opt.domain = attr.domain;
       Debug("http_tproxy", "Primary listen socket transparency is %s\n",
-	    attr.f_inbound_transparent &&  attr.f_outbound_transparent ? "bidirectional"
-	      : attr.f_inbound_transparent ? "inbound"
-	        : attr.f_outbound_transparent ? "outbound"
-                  : "off"
-	    );
+            attr.f_inbound_transparent &&  attr.f_outbound_transparent ? "bidirectional"
+            : attr.f_inbound_transparent ? "inbound"
+            : attr.f_outbound_transparent ? "outbound"
+            : "off"
+            );
       opt.f_outbound_transparent = attr.f_outbound_transparent;
       opt.f_inbound_transparent = attr.f_inbound_transparent;
       xfree(attr_string);
@@ -312,8 +312,7 @@ start_HttpProxyServer(int fd, int port, 
     }
   }
   if (!http_port_attr_array) {
-    netProcessor.main_accept(NEW(new HttpAccept(type)), fd,  NULL, NULL, false,
-                             opt);
+    netProcessor.main_accept(NEW(new HttpAccept(type)), fd,  NULL, NULL, false, opt);
 
     if (http_other_port_array) {
       for (int i = 0; http_other_port_array[i].port != -1; i++) {
@@ -321,21 +320,17 @@ start_HttpProxyServer(int fd, int port, 
         if ((e.port<1) || (e.port> 65535))
           Warning("additional port out of range ignored: %d", e.port);
         else {
-	  opt.port = e.port;
-	  opt.f_outbound_transparent = e.f_outbound_transparent;
-          netProcessor.main_accept(NEW(new HttpAccept(e.type)),
-				   fd, NULL, NULL, false, opt
-				   );
-	}
+          opt.port = e.port;
+          opt.f_outbound_transparent = e.f_outbound_transparent;
+          netProcessor.main_accept(NEW(new HttpAccept(e.type)), fd, NULL, NULL, false, opt);
+        }
       }
     }
   } else {
     for (int i = 0; http_port_attr_array[i].fd != NO_FD; i++) {
       HttpPortEntry & e = http_port_attr_array[i];
       if (!e.fd) {
-        netProcessor.main_accept(NEW(new HttpAccept(type)),
-				 fd, NULL, NULL, false, opt
-				 );
+        netProcessor.main_accept(NEW(new HttpAccept(type)), fd, NULL, NULL, false, opt);
       }
     }
   }
@@ -345,8 +340,7 @@ start_HttpProxyServer(int fd, int port, 
   if (sslParam->getTerminationMode() & sslParam->SSL_TERM_MODE_CLIENT) {
     opt.reset();
     opt.port = sslParam->getAcceptPort();
-    sslNetProcessor.main_accept(NEW(new HttpAccept(SERVER_PORT_SSL)), ssl_fd, 
-				0, 0, false, opt);
+    sslNetProcessor.main_accept(NEW(new HttpAccept(SERVER_PORT_SSL)), ssl_fd, 0, 0, false, opt);
   }
 
   sslTerminationConfig.release(sslParam);
@@ -368,6 +362,5 @@ start_HttpProxyServerBackDoor(int port)
 {
   NetProcessor::AcceptOptions opt;
   opt.port = port;
-  netProcessor.main_accept(NEW(new HttpAccept(SERVER_PORT_DEFAULT, true)),
-			   NO_FD, 0, 0, false, opt);
+  netProcessor.main_accept(NEW(new HttpAccept(SERVER_PORT_DEFAULT, true)), NO_FD, 0, 0, false, opt);
 }