subversion-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From stef...@apache.org
Subject svn commit: r1544745 - in /subversion/trunk/subversion/svnserve: server.h svnserve.c
Date Sat, 23 Nov 2013 06:39:39 GMT
Author: stefan2
Date: Sat Nov 23 06:39:38 2013
New Revision: 1544745

URL: http://svn.apache.org/r1544745
Log:
Introduce a data structure that combines all state of an svnserve
connection, including repository, pools, socket and server baton.
Use this new struct to eliminate mode-specific code and structures -
except for tunneling mode.

This will enable us to "suspend" connections in later commits,
i.e. handling many connections using a limit number of threads
and open repositories.

* subversion/svnserve/server.h
  (connection_t): new type consolidating all connection-related info

* subversion/svnserve/svnserve.c
  (shared_pool_t): drop type superseded by connection_t
  (accept_connection): new function with most code taken from
                       sub_main()'s network request handling loop
  (attach_shared_pool,
   release_shared_pool): replaced by these ...
  (attach_connection,
   close_connection): ... new connection-level functions
  (serve_socket): take a connection_t as parameter
  (serve_thread_t): drop type superseded by connection_t
  (serve_thread): thread param is now a connection_t
  (sub_main): simplify
  (main): note how the pool cleanup simplifies sub_main

Modified:
    subversion/trunk/subversion/svnserve/server.h
    subversion/trunk/subversion/svnserve/svnserve.c

Modified: subversion/trunk/subversion/svnserve/server.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnserve/server.h?rev=1544745&r1=1544744&r2=1544745&view=diff
==============================================================================
--- subversion/trunk/subversion/svnserve/server.h (original)
+++ subversion/trunk/subversion/svnserve/server.h Sat Nov 23 06:39:38 2013
@@ -36,8 +36,10 @@ extern "C" {
 #include "svn_repos.h"
 #include "svn_ra_svn.h"
 
+#include "private/svn_atomic.h"
 #include "private/svn_mutex.h"
 #include "private/svn_repos_private.h"
+#include "private/svn_subr_private.h"
   
 enum username_case_type { CASE_FORCE_UPPER, CASE_FORCE_LOWER, CASE_ASIS };
 
@@ -148,6 +150,42 @@ typedef struct serve_params_t {
   svn_boolean_t vhost;
 } serve_params_t;
 
+/* This structure contains all data that describes a client / server
+   connection.  Their lifetime is separated from the thread-local
+   serving pools. */
+typedef struct connection_t
+{
+  /* socket return by accept() */
+  apr_socket_t *usock;
+
+  /* server-global parameters */
+  serve_params_t *params;
+
+  /* connection-specific objects */
+  server_baton_t *baton;
+
+  /* buffered connection object used by the marshaller */
+  svn_ra_svn_conn_t *conn;
+
+  /* memory pool for objects with connection lifetime */
+  apr_pool_t *pool;
+
+  /* source and ultimate destiny for POOL */
+  svn_root_pools__t *root_pools;
+  
+  /* Number of threads using the pool.
+     The pool passed to apr_thread_create can only be released when both
+
+        A: the call to apr_thread_create has returned to the calling thread
+        B: the new thread has started running and reached apr_thread_start_t
+
+     So we set the atomic counter to 2 then both the calling thread and
+     the new thread decrease it and when it reaches 0 the pool can be
+     released.  */
+  svn_atomic_t ref_count;
+  
+} connection_t;
+
 /* Return a client_info_t structure allocated in POOL and initialize it
  * with data from CONN. */
 client_info_t * get_client_info(svn_ra_svn_conn_t *conn,

Modified: subversion/trunk/subversion/svnserve/svnserve.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/svnserve/svnserve.c?rev=1544745&r1=1544744&r2=1544745&view=diff
==============================================================================
--- subversion/trunk/subversion/svnserve/svnserve.c (original)
+++ subversion/trunk/subversion/svnserve/svnserve.c Sat Nov 23 06:39:38 2013
@@ -444,41 +444,78 @@ static apr_status_t redirect_stdout(void
   return apr_file_dup2(out_file, err_file, pool);
 }
 
-#if APR_HAS_THREADS
-/* The pool passed to apr_thread_create can only be released when both
-
-      A: the call to apr_thread_create has returned to the calling thread
-      B: the new thread has started running and reached apr_thread_start_t
-
-   So we set the atomic counter to 2 then both the calling thread and
-   the new thread decrease it and when it reaches 0 the pool can be
-   released.  */
-typedef struct shared_pool_t {
-  svn_atomic_t count;
-  apr_pool_t *pool;              /* root pool used to allocate the socket */
-  svn_root_pools__t *root_pools; /* put it back into this after use */
-} shared_pool_t;
-
-static shared_pool_t *
-attach_shared_pool(apr_pool_t *pool,
-                   svn_root_pools__t *root_pools)
+/* Wait for the next client connection to come in from SOCK.  Allocate
+ * the connection in a root pool from CONNECTION_POOLS and assign PARAMS.
+ * Return the connection object in *CONNECTION.
+ * 
+ * Use HANDLING_MODE for proper internal cleanup.
+ */
+static svn_error_t *
+accept_connection(connection_t **connection,
+                  apr_socket_t *sock,
+                  svn_root_pools__t *connection_pools,
+                  serve_params_t *params,
+                  enum connection_handling_mode handling_mode)
 {
-  shared_pool_t *shared = apr_palloc(pool, sizeof(*shared));
-
-  shared->pool = pool;
-  shared->root_pools = root_pools;
-  svn_atomic_set(&shared->count, 2);
+  apr_status_t status;
+  
+  /* Non-standard pool handling.  The main thread never blocks to join
+   *         the connection threads so it cannot clean up after each one.  So
+   *         separate pools that can be cleared at thread exit are used. */
+  
+  apr_pool_t *pool = svn_root_pools__acquire_pool(connection_pools);
+  *connection = apr_pcalloc(pool, sizeof(**connection));
+  (*connection)->pool = pool;
+  (*connection)->params = params;
+  (*connection)->root_pools = connection_pools;
+  (*connection)->ref_count = 1;
+  
+  do
+    {
+      #ifdef WIN32
+      if (winservice_is_stopping())
+        exit(0);
+      #endif
+      
+      status = apr_socket_accept(&(*connection)->usock, sock, pool);
+      if (handling_mode == connection_mode_fork)
+        {
+          apr_proc_t proc;
+          
+          /* Collect any zombie child processes. */
+          while (apr_proc_wait_all_procs(&proc, NULL, NULL, APR_NOWAIT,
+            pool) == APR_CHILD_DONE)
+            ;
+        }
+    }
+  while (APR_STATUS_IS_EINTR(status)
+    || APR_STATUS_IS_ECONNABORTED(status)
+    || APR_STATUS_IS_ECONNRESET(status));
+  
+  return status
+       ? svn_error_wrap_apr(status, _("Can't accept client connection"))
+       : SVN_NO_ERROR;
+}
 
-  return shared;
+/* Add a reference to CONNECTION, i.e. keep it and it's pool valid unless
+ * that reference gets released using release_shared_pool().
+ */
+static void
+attach_connection(connection_t *connection)
+{
+  svn_atomic_inc(&connection->ref_count);
 }
 
+/* Release a reference to CONNECTION.  If there are no more references,
+ * the connection will be
+ */
 static void
-release_shared_pool(struct shared_pool_t *shared)
+close_connection(connection_t *connection)
 {
-  if (svn_atomic_dec(&shared->count) == 0)
-    svn_root_pools__release_pool(shared->pool, shared->root_pools);
+  /* this will automatically close USOCK */
+  if (svn_atomic_dec(&connection->ref_count) == 0)
+    svn_root_pools__release_pool(connection->pool, connection->root_pools);
 }
-#endif
 
 /* Wrapper around serve() that takes a socket instead of a connection.
  * This is to off-load work from the main thread in threaded and fork modes.
@@ -486,12 +523,10 @@ release_shared_pool(struct shared_pool_t
  * If an error occurs, log it and also return it.
  */
 static svn_error_t *
-serve_socket(apr_socket_t *usock,
-             serve_params_t *params,
+serve_socket(connection_t *connection,
              apr_pool_t *pool)
 {
   apr_status_t status;
-  svn_ra_svn_conn_t *conn;
   svn_error_t *err;
   
   /* Enable TCP keep-alives on the socket so we time out when
@@ -502,37 +537,30 @@ serve_socket(apr_socket_t *usock,
    * it will respond to the keep-alive probe with a RST instead of an
    * acknowledgment segment, which will cause svn to abort the session
    * even while it is currently blocked waiting for data from the peer. */
-  status = apr_socket_opt_set(usock, APR_SO_KEEPALIVE, 1);
+  status = apr_socket_opt_set(connection->usock, APR_SO_KEEPALIVE, 1);
   if (status)
     {
       /* It's not a fatal error if we cannot enable keep-alives. */
     }
 
   /* create the connection, configure ports etc. */
-  conn = svn_ra_svn_create_conn3(usock, NULL, NULL,
-                                 params->compression_level,
-                                 params->zero_copy_limit,
-                                 params->error_check_interval,
-                                 pool);
+  connection->conn
+    = svn_ra_svn_create_conn3(connection->usock, NULL, NULL,
+                              connection->params->compression_level,
+                              connection->params->zero_copy_limit,
+                              connection->params->error_check_interval,
+                              pool);
 
   /* process the actual request and log errors */
-  err = serve(conn, params, pool);
+  err = serve(connection->conn, connection->params, pool);
   if (err)
-    logger__log_error(params->logger, err, NULL,
-                      get_client_info(conn, params, pool));
+    logger__log_error(connection->params->logger, err, NULL,
+                      get_client_info(connection->conn, connection->params,
+                                      pool));
 
   return svn_error_trace(err);
 }
 
-/* "Arguments" passed from the main thread to the connection thread */
-struct serve_thread_t {
-  apr_socket_t *usock;
-  serve_params_t *params;
-#if APR_HAS_THREADS
-  shared_pool_t *shared_pool;
-#endif
-};
-
 #if APR_HAS_THREADS
 
 /* allocate and recycle root pools for connection objects.
@@ -541,14 +569,16 @@ svn_root_pools__t *connection_pools;
 
 static void * APR_THREAD_FUNC serve_thread(apr_thread_t *tid, void *data)
 {
-  struct serve_thread_t *d = data;
-
+  struct connection_t *connection = data;
   apr_pool_t *pool = svn_root_pools__acquire_pool(connection_pools);
+
   /* serve_socket() logs any error it returns, so ignore it. */
-  svn_error_clear(serve_socket(d->usock, d->params, pool));
+  svn_error_clear(serve_socket(connection, pool));
+
   svn_root_pools__release_pool(pool, connection_pools);
 
-  release_shared_pool(d->shared_pool);
+  /* destroy the connection object */
+  close_connection(connection);
 
   return NULL;
 }
@@ -603,7 +633,7 @@ sub_main(int *exit_code, int argc, const
 {
   enum run_mode run_mode = run_mode_unspecified;
   svn_boolean_t foreground = FALSE;
-  apr_socket_t *sock, *usock;
+  apr_socket_t *sock;
   apr_file_t *in_file, *out_file;
   apr_sockaddr_t *sa;
   svn_error_t *err;
@@ -614,8 +644,6 @@ sub_main(int *exit_code, int argc, const
   apr_status_t status;
   apr_proc_t proc;
 #if APR_HAS_THREADS
-  shared_pool_t *shared_pool;
-  struct serve_thread_t *thread_data;
 #if HAVE_THREADPOOLS
   apr_thread_pool_t *threads;
 #else
@@ -1207,46 +1235,13 @@ sub_main(int *exit_code, int argc, const
 
   while (1)
     {
-      apr_pool_t *socket_pool;
-
-#ifdef WIN32
-      if (winservice_is_stopping())
-        return ERROR_SUCCESS;
-#endif
-
-      /* Non-standard pool handling.  The main thread never blocks to join
-         the connection threads so it cannot clean up after each one.  So
-         separate pools that can be cleared at thread exit are used. */
-
-      socket_pool = svn_root_pools__acquire_pool(socket_pools);
-
-      status = apr_socket_accept(&usock, sock, socket_pool);
-      if (handling_mode == connection_mode_fork)
-        {
-          /* Collect any zombie child processes. */
-          while (apr_proc_wait_all_procs(&proc, NULL, NULL, APR_NOWAIT,
-                                         socket_pool) == APR_CHILD_DONE)
-            ;
-        }
-      if (APR_STATUS_IS_EINTR(status)
-          || APR_STATUS_IS_ECONNABORTED(status)
-          || APR_STATUS_IS_ECONNRESET(status))
-        {
-          svn_root_pools__release_pool(socket_pool, socket_pools);
-          continue;
-        }
-      if (status)
-        {
-          return svn_error_wrap_apr(status,
-                                    _("Can't accept client connection"));
-        }
-
+      connection_t *connection = NULL;
+      SVN_ERR(accept_connection(&connection, sock, socket_pools, &params,
+                                handling_mode));
       if (run_mode == run_mode_listen_once)
         {
-          err = serve_socket(usock, &params, socket_pool);
-
-          apr_socket_close(usock);
-          apr_socket_close(sock);
+          err = serve_socket(connection, connection->pool);
+          close_connection(connection);
           return err;
         }
 
@@ -1254,27 +1249,23 @@ sub_main(int *exit_code, int argc, const
         {
         case connection_mode_fork:
 #if APR_HAS_FORK
-          status = apr_proc_fork(&proc, socket_pool);
+          status = apr_proc_fork(&proc, connection->pool);
           if (status == APR_INCHILD)
             {
+              /* the child would't listen to the main server's socket */
               apr_socket_close(sock);
+
               /* serve_socket() logs any error it returns, so ignore it. */
-              svn_error_clear(serve_socket(usock, &params, socket_pool));
-              apr_socket_close(usock);
+              svn_error_clear(serve_socket(connection, connection->pool));
+              close_connection(connection);
               return SVN_NO_ERROR;
             }
-          else if (status == APR_INPARENT)
-            {
-              apr_socket_close(usock);
-            }
-          else
+          else if (status != APR_INPARENT)
             {
               err = svn_error_wrap_apr(status, "apr_proc_fork");
               logger__log_error(params.logger, err, NULL, NULL);
               svn_error_clear(err);
-              apr_socket_close(usock);
             }
-          svn_root_pools__release_pool(socket_pool, socket_pools);
 #endif
           break;
 
@@ -1283,14 +1274,10 @@ sub_main(int *exit_code, int argc, const
              particularly sophisticated strategy for a threaded server, it's
              little different from forking one process per connection. */
 #if APR_HAS_THREADS
-          shared_pool = attach_shared_pool(socket_pool, socket_pools);
+          attach_connection(connection);
 
-          thread_data = apr_palloc(socket_pool, sizeof(*thread_data));
-          thread_data->usock = usock;
-          thread_data->params = &params;
-          thread_data->shared_pool = shared_pool;
 #if HAVE_THREADPOOLS
-          status = apr_thread_pool_push(threads, serve_thread, thread_data,
+          status = apr_thread_pool_push(threads, serve_thread, connection,
                                         0, NULL);
 #else
           status = apr_threadattr_create(&tattr, socket_pool);
@@ -1310,16 +1297,16 @@ sub_main(int *exit_code, int argc, const
             {
               return svn_error_wrap_apr(status, THREAD_ERROR_MSG);
             }
-          release_shared_pool(shared_pool);
 #endif
           break;
 
         case connection_mode_single:
           /* Serve one connection at a time. */
           /* serve_socket() logs any error it returns, so ignore it. */
-          svn_error_clear(serve_socket(usock, &params, socket_pool));
-          svn_root_pools__release_pool(socket_pool, socket_pools);
+          svn_error_clear(serve_socket(connection, connection->pool));
         }
+
+      close_connection(connection);
     }
 
   /* NOTREACHED */
@@ -1351,6 +1338,7 @@ main(int argc, const char *argv[])
       svn_cmdline_handle_exit_error(err, NULL, "svnserve: ");
     }
 
+  /* this will also close the server's socket */
   svn_pool_destroy(pool);
   return exit_code;
 }



Mime
View raw message