From 77804c1a2ef2a8e000a9abf9a85e7e4d765e252f Mon Sep 17 00:00:00 2001 From: Dan Winship Date: Tue, 16 Aug 2005 14:33:42 +0000 Subject: Fix a connection leak reported by Tambet. * libsoup/soup-connection.c (send_request): rather than tracking the message progress via signals, call soup_message_send_request_internal() and have it call soup_connection_release() when it's done. (request_restarted, request_done): gone (clear_current_request): handle disconnecting (if necessary) and updating last_used time here. (soup_connection_release): Call clear_current_request(). (dispose): Call clear_current_request() * libsoup/soup-message-client-io.c (soup_message_send_request_internal): New. Takes a SoupConnection in addition to the other args, and passes that on to soup-message-io. * libsoup/soup-message-io.c (SoupMessageIOData): add a SoupConnection field. (io_cleanup): if io->conn is set, unref it. (soup_message_io_stop): if io->conn is set, and we ended in a clean state, call soup_connection_release() on it. (soup_message_io_client): Add a SoupConnection arg, which gets reffed and stored in io->conn. * TODO: misc updates --- ChangeLog | 29 +++++++++++++++++++++ TODO | 40 ++++++++++++++-------------- libsoup/soup-connection.c | 56 +++++++++++----------------------------- libsoup/soup-message-client-io.c | 9 ++++++- libsoup/soup-message-io.c | 11 ++++++++ libsoup/soup-message-private.h | 6 +++++ 6 files changed, 88 insertions(+), 63 deletions(-) diff --git a/ChangeLog b/ChangeLog index 32fc105b..2a3331e9 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,32 @@ +2005-08-16 Dan Winship + + Fix a connection leak reported by Tambet. + + * libsoup/soup-connection.c (send_request): rather than tracking + the message progress via signals, call + soup_message_send_request_internal() and have it call + soup_connection_release() when it's done. + (request_restarted, request_done): gone + (clear_current_request): handle disconnecting (if necessary) and + updating last_used time here. + (soup_connection_release): Call clear_current_request(). + (dispose): Call clear_current_request() + + * libsoup/soup-message-client-io.c + (soup_message_send_request_internal): New. Takes a SoupConnection + in addition to the other args, and passes that on to + soup-message-io. + + * libsoup/soup-message-io.c (SoupMessageIOData): add a + SoupConnection field. + (io_cleanup): if io->conn is set, unref it. + (soup_message_io_stop): if io->conn is set, and we ended in a + clean state, call soup_connection_release() on it. + (soup_message_io_client): Add a SoupConnection arg, which gets + reffed and stored in io->conn. + + * TODO: misc updates + 2005-08-15 Dan Winship * libsoup/soup-connection.h (soup_connection_new): diff --git a/TODO b/TODO index 89791567..1bbe8781 100644 --- a/TODO +++ b/TODO @@ -10,6 +10,21 @@ Known bugs/warts to fix * Expect: 100-continue processing doesn't currently abort sending the body if it gets back a non-informational status code +* SoupConnections never time themselves out. They always wait for the + server to time them out (and even then they don't time out until you + try to use them again). + * NTLM connections should stay open longer than plain ones + +* URI encoding bugs + * query component at least, maybe others must be stored + fully-escaped, because only the endpoints know which + chars are and aren't reserved + * When fixing this, also consider: + * Merging E2kUri with SoupUri + * application/x-www-form-urlencoded, which encodes + space as "+" rather than "%20" + * SoupUri doesn't currently allow "*" as a URI + HTTP Features ------------- @@ -17,6 +32,8 @@ HTTP Features * Handle gzip Content-Encoding * Handle cookies (RFC 2109) via a SoupCookieJar message filter + * libexchange has some minimal cookie functionality for + Exchange 2003 forms-based auth * RFC 2965 supposedly obsoletes 2109, but I don't think anyone uses it @@ -55,15 +72,7 @@ Misc Features General/API stuff ----------------- -* Thread safety - * This is mostly done, but need to figure out what other - things need to be thread-safe, and document which ones - aren't. - -* Documentation (API, tutorial) - * gtk-doc is not currently picking up all of the objects. - This probably has something to do with the fact that - some of the typdefs are in soup-types.h. +* Documentation (tutorial, hacking, thread safety) * User-Agent handling: caller should specify a User-Agent string to SoupSession, and soup should automatically append libsoup/#.## to @@ -94,24 +103,13 @@ General/API stuff * Make it possible to implement CONNECT on the server side (by having a way to steal the socket from the SoupServer). -* Add date-parsing/generating routines (all HTTP-specified formats) - * Special handling on server side for HEAD (don't send response.body). * Simple higher-level API (a la E2kContext) * the Connector stuff has a lot of Exchange-specific assumptions and exceptions. Might not be possible to migrate Connector to a generic API - -* URI encoding bugs - * query component at least, maybe others must be stored - fully-escaped, because only the endpoints know which - chars are and aren't reserved - * When fixing this, also consider: - * Merging E2kUri with SoupUri - * application/x-www-form-urlencoded, which encodes - space as "+" rather than "%20" - * SoupUri doesn't currently allow "*" as a URI + * jamesh's HttpResource object Testing diff --git a/libsoup/soup-connection.c b/libsoup/soup-connection.c index a38f05b3..d54ec6cb 100644 --- a/libsoup/soup-connection.c +++ b/libsoup/soup-connection.c @@ -21,6 +21,7 @@ #include "soup-connection.h" #include "soup-marshal.h" #include "soup-message.h" +#include "soup-message-private.h" #include "soup-message-filter.h" #include "soup-misc.h" #include "soup-socket.h" @@ -75,8 +76,8 @@ static void set_property (GObject *object, guint prop_id, static void get_property (GObject *object, guint prop_id, GValue *value, GParamSpec *pspec); -static void request_done (SoupMessage *req, gpointer user_data); static void send_request (SoupConnection *conn, SoupMessage *req); +static void clear_current_request (SoupConnection *conn); static void soup_connection_init (SoupConnection *conn) @@ -103,10 +104,8 @@ static void dispose (GObject *object) { SoupConnection *conn = SOUP_CONNECTION (object); - SoupConnectionPrivate *priv = SOUP_CONNECTION_GET_PRIVATE (object); - if (priv->cur_req) - request_done (priv->cur_req, conn); + clear_current_request (conn); soup_connection_disconnect (conn); G_OBJECT_CLASS (soup_connection_parent_class)->dispose (object); @@ -351,9 +350,16 @@ set_current_request (SoupConnectionPrivate *priv, SoupMessage *req) } static void -clear_current_request (SoupConnectionPrivate *priv) +clear_current_request (SoupConnection *conn) { + SoupConnectionPrivate *priv = SOUP_CONNECTION_GET_PRIVATE (conn); + if (priv->cur_req) { + if (!soup_message_is_keepalive (priv->cur_req)) + soup_connection_disconnect (conn); + else + priv->last_used = time (NULL); + g_object_remove_weak_pointer (G_OBJECT (priv->cur_req), (gpointer *)priv->cur_req); priv->cur_req = NULL; @@ -388,7 +394,7 @@ tunnel_connect_finished (SoupMessage *msg, gpointer user_data) SoupConnectionPrivate *priv = SOUP_CONNECTION_GET_PRIVATE (conn); guint status = msg->status_code; - clear_current_request (priv); + clear_current_request (conn); if (SOUP_STATUS_IS_SUCCESSFUL (status)) { if (soup_socket_start_proxy_ssl (priv->socket, @@ -695,51 +701,19 @@ soup_connection_last_used (SoupConnection *conn) return SOUP_CONNECTION_GET_PRIVATE (conn)->last_used; } -static void -request_restarted (SoupMessage *req, gpointer conn) -{ - if (!soup_message_is_keepalive (req)) - soup_connection_disconnect (conn); -} - -static void -request_done (SoupMessage *req, gpointer user_data) -{ - SoupConnection *conn = user_data; - SoupConnectionPrivate *priv = SOUP_CONNECTION_GET_PRIVATE (conn); - - clear_current_request (priv); - priv->last_used = time (NULL); - - if (!soup_message_is_keepalive (req)) - soup_connection_disconnect (conn); - - g_signal_handlers_disconnect_by_func (req, request_done, conn); - g_signal_handlers_disconnect_by_func (req, request_restarted, conn); - g_object_unref (conn); -} - static void send_request (SoupConnection *conn, SoupMessage *req) { SoupConnectionPrivate *priv = SOUP_CONNECTION_GET_PRIVATE (conn); - g_object_ref (conn); - if (req != priv->cur_req) { set_current_request (priv, req); - - g_signal_connect (req, "restarted", - G_CALLBACK (request_restarted), conn); - g_signal_connect (req, "finished", - G_CALLBACK (request_done), conn); - if (priv->filter) soup_message_filter_setup_message (priv->filter, req); } - soup_message_send_request (req, priv->socket, - priv->proxy_uri != NULL); + soup_message_send_request_internal (req, priv->socket, conn, + priv->proxy_uri != NULL); } /** @@ -790,7 +764,7 @@ soup_connection_release (SoupConnection *conn) { g_return_if_fail (SOUP_IS_CONNECTION (conn)); - SOUP_CONNECTION_GET_PRIVATE (conn)->in_use = FALSE; + clear_current_request (conn); } /** diff --git a/libsoup/soup-message-client-io.c b/libsoup/soup-message-client-io.c index 1cad9f63..86b68d64 100644 --- a/libsoup/soup-message-client-io.c +++ b/libsoup/soup-message-client-io.c @@ -173,9 +173,16 @@ get_request_headers (SoupMessage *req, GString *header, void soup_message_send_request (SoupMessage *req, SoupSocket *sock, gboolean is_via_proxy) +{ + soup_message_send_request_internal (req, sock, NULL, is_via_proxy); +} + +void +soup_message_send_request_internal (SoupMessage *req, SoupSocket *sock, + SoupConnection *conn, gboolean is_via_proxy) { soup_message_cleanup_response (req); - soup_message_io_client (req, sock, + soup_message_io_client (req, sock, conn, get_request_headers, parse_response_headers, GUINT_TO_POINTER (is_via_proxy)); diff --git a/libsoup/soup-message-io.c b/libsoup/soup-message-io.c index b551a277..427c4d48 100644 --- a/libsoup/soup-message-io.c +++ b/libsoup/soup-message-io.c @@ -12,6 +12,7 @@ #include #include +#include "soup-connection.h" #include "soup-message.h" #include "soup-message-private.h" #include "soup-socket.h" @@ -36,6 +37,7 @@ typedef enum { typedef struct { SoupSocket *sock; + SoupConnection *conn; SoupMessageIOMode mode; SoupMessageIOState read_state; @@ -82,6 +84,8 @@ io_cleanup (SoupMessage *msg) if (io->sock) g_object_unref (io->sock); + if (io->conn) + g_object_unref (io->conn); if (io->read_buf) g_byte_array_free (io->read_buf, TRUE); @@ -124,6 +128,8 @@ soup_message_io_stop (SoupMessage *msg) if (io->read_state != SOUP_MESSAGE_IO_STATE_DONE) soup_socket_disconnect (io->sock); + else if (io->conn) + soup_connection_release (io->conn); } #define SOUP_MESSAGE_IO_EOL "\r\n" @@ -707,6 +713,7 @@ new_iostate (SoupMessage *msg, SoupSocket *sock, SoupMessageIOMode mode, * soup_message_io_client: * @msg: a #SoupMessage * @sock: socket to send @msg across + * @conn: the connection that owns @sock (or %NULL) * @get_headers_cb: callback function to generate request headers * @parse_headers_cb: callback function to parse response headers * @user_data: data to pass to the callbacks @@ -717,6 +724,7 @@ new_iostate (SoupMessage *msg, SoupSocket *sock, SoupMessageIOMode mode, **/ void soup_message_io_client (SoupMessage *msg, SoupSocket *sock, + SoupConnection *conn, SoupMessageGetHeadersFn get_headers_cb, SoupMessageParseHeadersFn parse_headers_cb, gpointer user_data) @@ -726,6 +734,9 @@ soup_message_io_client (SoupMessage *msg, SoupSocket *sock, io = new_iostate (msg, sock, SOUP_MESSAGE_IO_CLIENT, get_headers_cb, parse_headers_cb, user_data); + if (conn) + io->conn = g_object_ref (conn); + io->read_body = &msg->response; io->write_body = &msg->request; diff --git a/libsoup/soup-message-private.h b/libsoup/soup-message-private.h index e1168f6d..04ab11b5 100644 --- a/libsoup/soup-message-private.h +++ b/libsoup/soup-message-private.h @@ -40,8 +40,14 @@ typedef guint (*SoupMessageParseHeadersFn)(SoupMessage *msg, guint *content_len, gpointer user_data); +void soup_message_send_request_internal (SoupMessage *req, + SoupSocket *sock, + SoupConnection *conn, + gboolean via_proxy); + void soup_message_io_client (SoupMessage *msg, SoupSocket *sock, + SoupConnection *conn, SoupMessageGetHeadersFn get_headers_cb, SoupMessageParseHeadersFn parse_headers_cb, gpointer user_data); -- cgit v1.2.1