GNOME Bugzilla – Bug 638576
Please add a way to explicitly shut down a server
Last modified: 2011-03-12 19:34:37 UTC
In C, you can cause a server to close its socket by destroying the server object using `g_object_unref'. However, in language bindings, you normally don't have explicit control over the reference counting mechanism, as it is hidden and automatically managed by tying it to the language's garbage collector. Thus, with the current libsoup API, it is not possible to ensure the server's socket is shut down at a specific point in time, making it impossible to instantiate a new server on the same port or to make sure the server's socket is closed properly on program termination. My suggested solution is to add a new method `soup_server_shutdown' implementing that functionality for use in language bindings. I'll attach a patch that implements that method.
Created attachment 177411 [details] [review] Add `soup_server_shutdown' message
Comment on attachment 177411 [details] [review] Add `soup_server_shutdown' message Yeah, this makes sense. >See <https://bugzilla.gnome.org/show_bug.cgi?id=638576>. Just put the URI alone (no <> or .), for consistency with other commit messages. (Also, don't use un-smart-quotes; "`" doesn't draw as a left quote in any modern font. Just use "'" on both sides.) >+ * soup_server_shutdown: It would be nice to have name that isn't as ambiguous with soup_server_quit(). (Of course, it's soup_server_quit() that ought to be renamed probably, but we can't do that now...) Maybe soup_server_quit_and_shutdown() ? Or soup_server_disconnect() ? >+ * Stops processing for @server and closes its socket. This implies >+ * the effects of soup_server_quit(), but additionally closes the >+ * listening socket. >+ * >+ * After calling this function, @server is no longer functional, so it >+ * has nearly the same effect as destroying @server entirely. The >+ * function is thus useful mainly for language bindings without >+ * explicit control over object livetime. typo in the last line ("lifetime") It may be worth explicitly mentioning that messages currently-in-progress will continue being processed? (Or maybe it should actively kill them? In that case, soup_server_abort() might be a good name, for parallelism with soup_session_abort(). This would require some rewriting though, since SoupServer doesn't currently keep explicit track of messages-in-progress.) >+ if (priv->listen_sock) { >+ g_object_unref (priv->listen_sock); >+ priv->listen_sock = NULL; >+ } You should explicitly call soup_socket_disconnect() on it; the app may have previously called soup_session_get_listener() in which case the language binding may also be holding a ref on the socket itself.
Created attachment 182018 [details] [review] Add 'soup_server_shutdown' message Applied suggestions from comment #2.
(In reply to comment #2) > (From update of attachment 177411 [details] [review]) > Yeah, this makes sense. > > >See <https://bugzilla.gnome.org/show_bug.cgi?id=638576>. > > Just put the URI alone (no <> or .), for consistency with other commit > messages. > > (Also, don't use un-smart-quotes; "`" doesn't draw as a left quote in any > modern font. Just use "'" on both sides.) > Done. > >+ * soup_server_shutdown: > > It would be nice to have name that isn't as ambiguous with soup_server_quit(). > (Of course, it's soup_server_quit() that ought to be renamed probably, but we > can't do that now...) > > Maybe soup_server_quit_and_shutdown() ? Or soup_server_disconnect() ? > soup_server_disconnect() seems fine, adapted the patch accordingly. > typo in the last line ("lifetime") > Thanks, fixed. > It may be worth explicitly mentioning that messages currently-in-progress will > continue being processed? > I was not aware that this would be the case; in my current use-case, it doesn't matter, as I terminate the process after calling that method. I have added a note about that behavior to the documentation of soup_server_disconnect(). > (Or maybe it should actively kill them? In that case, > soup_server_abort() might be a good name, for parallelism with > soup_session_abort(). This would require some rewriting though, since > SoupServer doesn't currently keep explicit track of messages-in-progress.) > Perhaps killing currently active messages should be an orthogonal operation, in case it is useful in other situations? > >+ if (priv->listen_sock) { > >+ g_object_unref (priv->listen_sock); > >+ priv->listen_sock = NULL; > >+ } > > You should explicitly call soup_socket_disconnect() on it; the app may have > previously called soup_session_get_listener() in which case the language > binding may also be holding a ref on the socket itself. > Done.
Comment on attachment 182018 [details] [review] Add 'soup_server_shutdown' message looks good. ship it. (or do you not have a gnome git account?)
Committed and pushed (af48c7160..).