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