[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

issue-2382 branch (was: Re: Status of outstanding branches)

From: Stefan Sperling <stsp_at_elego.de>
Date: Tue, 17 Feb 2009 07:27:22 +0000

On Mon, Feb 16, 2009 at 04:55:59PM -0600, Hyrum Wright wrote:
> We've branched 1.6.x: yay! While it's soaking, and tion for the 1.7
> release cycle, I'd like to take a look at all the current branches in
> our tree, find out what's mergable and what's not, and then take
> appropriate action. Here's the list of the outstanding branches in
> our tree:
 
> * issue-2382/
> 3-month-old branch created by Stefan Sperling to "handle dual-stack
> IPv4/IPv6 servers in svnserve."

I'd still like to complete this. svnserve does not work with IPv6 as is,
or more precisely, it does not work on dual-stack hosts (which in
practice means that it does not properly support IPv6).

I got stuck trying to poll multiple sockets via APR.
httpd does this successfully.

The patch below shows what I have in queue for this branch to finish it off.
Only about half the connection attempts to svnserve get through with
this. I don't know what I'm doing wrong.

Stefan

Index: subversion/svnserve/listen.c
===================================================================
--- subversion/svnserve/listen.c (revision 33766)
+++ subversion/svnserve/listen.c (working copy)
@@ -18,6 +18,7 @@
 
 #include <apr_tables.h>
 #include <apr_network_io.h>
+#include <apr_poll.h>
 
 #include "svn_types.h"
 #include "svn_pools.h"
@@ -111,6 +112,7 @@ parse_addresses(apr_array_header_t **parsed_addres
 }
 
 svn_error_t* init_listeners(apr_array_header_t **listeners,
+ apr_pollset_t **pollset,
                             apr_array_header_t *addresses,
                             apr_pool_t *pool)
 {
@@ -120,10 +122,8 @@ svn_error_t* init_listeners(apr_array_header_t **l
   apr_socket_t *sock;
   apr_array_header_t *parsed_addresses;
   apr_pool_t *parse_pool;
- apr_array_header_t *new_listeners;
 
- *listeners = NULL;
- new_listeners = apr_array_make(pool, 1, sizeof(struct listener));
+ *listeners = apr_array_make(pool, 1, sizeof(struct listener *));
 
 #if APR_HAVE_IPV6
   /* Make sure we have IPV6 support first before giving apr_sockaddr_info_get
@@ -198,21 +198,62 @@ svn_error_t* init_listeners(apr_array_header_t **l
           listener = apr_palloc(pool, sizeof(struct listener));
           listener->sock = sock;
           listener->sa = sa;
- APR_ARRAY_PUSH(new_listeners, struct listener *) = listener;
+ status = apr_socket_listen(listener->sock, CONNECTION_BACKLOG);
+ if (status)
+ return svn_error_wrap_apr(status, _("Cannot listen on socket"));
+ APR_ARRAY_PUSH(*listeners, struct listener *) = listener;
 
           sa = sa->next;
         }
     }
 
- *listeners = new_listeners;
+ svn_pool_destroy(parse_pool);
 
- svn_pool_destroy(parse_pool);
+ if ((*listeners)->nelts > 1)
+ {
+ /* We'll need to poll multiple sockets, so create a pollset. */
+ status = apr_pollset_create(pollset, (*listeners)->nelts, pool, 0);
+ if (status)
+ return svn_error_wrap_apr(status, _("Cannot create pollset"));
+
+ /* Add all listeners to the pollset. */
+ for (i = 0; i < (*listeners)->nelts; i++)
+ {
+ apr_pollfd_t pollfd;
+ struct listener *listener;
+
+ listener = APR_ARRAY_IDX(*listeners, i, struct listener *);
+
+ /* Mark socket as non-blocking. */
+ status = apr_socket_opt_set(listener->sock, APR_SO_NONBLOCK, 1);
+ if (status)
+ return svn_error_wrap_apr(status,
+ _("Cannot mark socket as non-blocking"));
+
+ /* Why do I have to set up pollfds manually???
+ * APR needs apr_pollfd_create() or something like that... */
+ memset(&pollfd, 0, sizeof(apr_pollfd_t));
+ pollfd.desc_type = APR_POLL_SOCKET;
+ pollfd.desc.s = listener->sock;
+ pollfd.reqevents = APR_POLLIN;
+ pollfd.client_data = listener;
+
+ status = apr_pollset_add(*pollset, &pollfd);
+ if (status)
+ return svn_error_wrap_apr(status, _("Cannot add to pollset"));
+
+ }
+ }
+ else
+ *pollset = NULL;
+
   return SVN_NO_ERROR;
 }
 
 svn_error_t *
 wait_for_client(apr_socket_t **usock,
                 apr_array_header_t *listeners,
+ apr_pollset_t *pollset,
                 apr_pool_t *pool)
 {
   struct listener *listener;
@@ -224,19 +265,38 @@ wait_for_client(apr_socket_t **usock,
   /* Straightforward case: If we have only one listener,
    * we do not need to poll across multiple sockets. */
   if (listeners->nelts == 1)
+ listener = APR_ARRAY_IDX(listeners, 0, struct listener *);
+ else
     {
- listener = APR_ARRAY_IDX(listeners, 0, struct listener *);
- status = apr_socket_listen(listener->sock, CONNECTION_BACKLOG);
- if (status)
- return svn_error_wrap_apr(status, _("Cannot listen on socket"));
- status = apr_socket_accept(usock, listener->sock, pool);
- if (status)
- return svn_error_wrap_apr(status, _("Cannot accept connection"));
- return SVN_NO_ERROR;
+ /* We need to poll multiple sockets. */
+ const apr_pollfd_t *pollfds;
+ apr_int32_t numpollfds;
+ static int pollfd = 0;
+
+ SVN_ERR_ASSERT(pollset);
+
+ status = apr_pollset_poll(pollset, -1, &numpollfds, &pollfds);
+ if (APR_STATUS_IS_EINTR(status))
+ return svn_error_wrap_apr(status, _("Got signal"));
+ else if (status)
+ return svn_error_wrap_apr(status, _("Cannot poll sockets"));
+
+ /* In case there is something happening on multiple descriptors,
+ * we go through them in a round-robin fashion (this is what
+ * httpd does, also). Keep the pollfd index we use for this
+ * in bounds. */
+ if (pollfd >= numpollfds)
+ pollfd = 0;
+
+ /* Grab the listener corresponding to the polled fd,
+ * and advance the pollfd index so we serve the next
+ * busy socket next time around. */
+ listener = pollfds[pollfd++].client_data;
     }
 
- /* TODO: multiple addresses */
- SVN_ERR_ASSERT(FALSE);
+ status = apr_socket_accept(usock, listener->sock, pool);
+ if (status)
+ return svn_error_wrap_apr(status, _("Cannot accept connection"));
 
   return SVN_NO_ERROR;
 }
Index: subversion/svnserve/main.c
===================================================================
--- subversion/svnserve/main.c (revision 33766)
+++ subversion/svnserve/main.c (working copy)
@@ -374,6 +374,7 @@ int main(int argc, const char *argv[])
   apr_socket_t *sock, *usock;
   apr_file_t *in_file, *out_file;
   apr_array_header_t *addresses, *listeners;
+ apr_pollset_t *pollset;
   apr_pool_t *pool;
   apr_pool_t *connection_pool;
   svn_error_t *err;
@@ -704,7 +705,7 @@ int main(int argc, const char *argv[])
 #endif
     }
 
- err = init_listeners(&listeners, addresses, pool);
+ err = init_listeners(&listeners, &pollset, addresses, pool);
   if (err)
     return svn_cmdline_handle_exit_error(err, pool, "svnserve: ");
 
@@ -754,7 +755,7 @@ int main(int argc, const char *argv[])
       connection_pool = svn_pool_create(NULL);
 
       /* Start accepting connections. */
- err = wait_for_client(&usock, listeners, pool);
+ err = wait_for_client(&usock, listeners, pollset, pool);
 
       if (handling_mode == connection_mode_fork)
         {
Index: subversion/svnserve/server.h
===================================================================
--- subversion/svnserve/server.h (revision 33766)
+++ subversion/svnserve/server.h (working copy)
@@ -22,6 +22,7 @@
 #define SERVER_H
 
 #include <apr_network_io.h>
+#include <apr_poll.h>
 
 #ifdef __cplusplus
 extern "C" {
@@ -151,7 +152,10 @@ log_error(svn_error_t *err, apr_file_t *log_file,
  * description of valid input.
  *
  * Return an array of initialised listeners in *LISTENERS.
- * This should be passed to wait_for_client(), see below.
+ * If more than one address is specified, *POLLSET will be
+ * set to an APR pollset used for polling sockets.
+ * Both *LISTENERS and *POLLSET should be passed to wait_for_client(),
+ * see below.
  *
  * If an address does not specify a hostname, the unspecified address
  * will be used (APR_ANYADDR for IPv4, "::" for IPv6).
@@ -165,6 +169,7 @@ log_error(svn_error_t *err, apr_file_t *log_file,
  * Do all allocations in POOL.
  */
 svn_error_t* init_listeners(apr_array_header_t **listeners,
+ apr_pollset_t **pollset,
                             apr_array_header_t *addresses,
                             apr_pool_t *pool);
 
@@ -172,14 +177,16 @@ svn_error_t* init_listeners(apr_array_header_t **l
  * for communication with the client in *USOCK.
  * When done, the caller should close *USOCK with apr_socket_close().
  *
- * LISTENERS is a pointer to an array of listeners obtained from
- * init_listeners() (see above).
+ * LISTENERS and POLLSET are pointers to an array of listeners and an
+ * APR pollset, respectively. They should be obtained from init_listeners()
+ * (see above).
  *
  * Do all allocations in POOL.
  */
 svn_error_t*
 wait_for_client(apr_socket_t **usock,
                 apr_array_header_t *listeners,
+ apr_pollset_t *pollset,
                 apr_pool_t *pool);
 
 #ifdef __cplusplus
Received on 2009-02-17 08:27:48 CET

This is an archived mail posted to the Subversion Dev mailing list.