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

Re: issue-2382 -- question about winservice_svnserve_accept_socket

From: D.J. Heap <djheap_at_gmail.com>
Date: Wed, 22 Oct 2008 13:03:43 -0600

On Sun, Oct 19, 2008 at 10:06 AM, Stefan Sperling <stsp_at_elego.de> wrote:
> Hey windows folks,
> I'm working on issue #2382 (Handle dual-stack IPv4/IPv6 servers in
> svnserve) on the aptly named issue-2382 branch.
> I'm trying to make svnserve listen on multiple sockets (an arbitrary
> amount, actually), so instead of a single server socket, there is
> now a list of sockets svnserve is listening on.
> Addresses to listen on can be specified like this:
> svnserve --listen localhost:5555 --listen svn.example.com:4242 \
> --listen svn.example.com --listen ...
> # ^ use default port
> subversion/svnserve/main.c contains the following snippet:
> --- snip ---
> #ifdef WIN32
> static apr_os_sock_t winservice_svnserve_accept_socket = INVALID_SOCKET;
> /* The SCM calls this function (on an arbitrary thread, not the main()
> thread!) when it wants to stop the service.
> For now, our strategy is to close the listener socket, in order to
> unblock main() and cause it to exit its accept loop. We cannot use
> apr_socket_close, because that function deletes the apr_socket_t
> structure, as well as closing the socket handle. If we called
> apr_socket_close here, then main() will also call apr_socket_close,
> resulting in a double-free. This way, we just close the kernel
> socket handle, which causes the accept() function call to fail,
> which causes main() to clean up the socket. So, memory gets freed
> only once.
> This isn't pretty, but it's better than a lot of other options.
> Currently, there is no "right" way to shut down svnserve.
> We store the OS handle rather than a pointer to the apr_socket_t
> structure in order to eliminate any possibility of illegal memory
> access. */
> void winservice_notify_stop(void)
> {
> if (winservice_svnserve_accept_socket != INVALID_SOCKET)
> closesocket(winservice_svnserve_accept_socket);
> }
> #endif /* _WIN32 */
> --- snap ---
> and further below we have this call:
> --- snip ---
> #ifdef WIN32
> status = apr_os_sock_get(&winservice_svnserve_accept_socket, sock);
> if (status)
> winservice_svnserve_accept_socket = INVALID_SOCKET;
> /* At this point, the service is "running". Notify the SCM. */
> if (run_mode == run_mode_service)
> winservice_running();
> #endif
> --- snip ---
> My question is, does it matter which socket is passed as the 'sock'
> parameter to apr_os_sock_get? Is a socket of the same type (ie. streamy)
> sufficient or should I pass the *exact* same socket in there that we are
> listening on (i.e. call apr_os_sock_get once for each socket)?
> And must I take special care to make this thread-safe?
> Can anyone firm in windows provide comments (and hopefully also test
> later, once the branch is done)?

In the current code it matters because the accept call blocks until a
client connects or the listening socket is closed.

On the branch code, it will depend on how the wait_for_client call
will work. If some provision is made to allow it to exit when a
shutdown signal is received, then there would be no reason to have
force the listener sockets closed at all. However, if wait_for_client
blocks until the listener sockets are closed or a client connects,
then we're in the same boat but with mulitple listeners.

The core issue is that main() needs to get control back from
wait_for_client when the service is signalled to shutdown so that it
can shutdown and exit.


To unsubscribe, e-mail: dev-unsubscribe_at_subversion.tigris.org
For additional commands, e-mail: dev-help_at_subversion.tigris.org
Received on 2008-10-22 21:03:58 CEST

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