You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Stefan Sperling <st...@elego.de> on 2008/10/19 16:06:29 UTC

issue-2382 -- question about winservice_svnserve_accept_socket

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)?

Thanks,
Stefan

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: issue-2382 -- question about winservice_svnserve_accept_socket

Posted by "D.J. Heap" <dj...@gmail.com>.
On Sun, Oct 19, 2008 at 10:06 AM, Stefan Sperling <st...@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.

DJ

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org