[libsoup/carlosgc/connection-state] connection: rework state changes
- From: Carlos Garcia Campos <carlosgc src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [libsoup/carlosgc/connection-state] connection: rework state changes
- Date: Tue, 11 May 2021 13:00:33 +0000 (UTC)
commit 6b9b7c4045b33ac4f61f3539a59c39536e21737a
Author: Carlos Garcia Campos <cgarcia igalia com>
Date: Tue May 11 14:47:48 2021 +0200
connection: rework state changes
Make soup_connection_set_state() private and the property read only. The
changes to IN_USE and IDLE states are now done by
soup_connection_set_in_use() that is called every time the connection is
assigned or unassigned to a message. When the connection is no longer
used by any message, it switches to IDLE state if it's reusable or
DISCONNECT otherwise. This simplifies the way the state is changed,
soup_connection_set_state() is now a simpler setter, it can't be called
recursively anymore. soup_connection_get_state() has been simplified
too, it was weird that a getter could also change the state, so it's now
a simpler getter that returns the current state. Callers can check if
the idle connection is still open if they need it by using
soup_connection_is_idle_open(). This removes the
SOUP_CONNECTION_REMOTE_DISCONNECTED state that is no longer used.
libsoup/soup-connection.c | 100 +++++++++++++++++++++++-----------------------
libsoup/soup-connection.h | 7 ++--
libsoup/soup-message.c | 2 +
libsoup/soup-session.c | 26 ++++--------
tests/connection-test.c | 1 -
5 files changed, 62 insertions(+), 74 deletions(-)
---
diff --git a/libsoup/soup-connection.c b/libsoup/soup-connection.c
index a7666246..c5283b6d 100644
--- a/libsoup/soup-connection.c
+++ b/libsoup/soup-connection.c
@@ -38,6 +38,7 @@ typedef struct {
SoupConnectionState state;
time_t unused_timeout;
GSource *idle_timeout_src;
+ guint in_use;
gboolean reusable;
GCancellable *cancellable;
@@ -135,9 +136,6 @@ soup_connection_set_property (GObject *object, guint prop_id,
case PROP_SOCKET_PROPERTIES:
priv->socket_props = g_value_dup_boxed (value);
break;
- case PROP_STATE:
- soup_connection_set_state (SOUP_CONNECTION (object), g_value_get_uint (value));
- break;
case PROP_SSL:
priv->ssl = g_value_get_boolean (value);
break;
@@ -254,8 +252,9 @@ soup_connection_class_init (SoupConnectionClass *connection_class)
g_param_spec_enum ("state",
"Connection state",
"Current state of connection",
- SOUP_TYPE_CONNECTION_STATE, SOUP_CONNECTION_NEW,
- G_PARAM_READWRITE |
+ SOUP_TYPE_CONNECTION_STATE,
+ SOUP_CONNECTION_NEW,
+ G_PARAM_READABLE |
G_PARAM_STATIC_STRINGS);
properties[PROP_SSL] =
g_param_spec_boolean ("ssl",
@@ -328,6 +327,22 @@ stop_idle_timer (SoupConnectionPrivate *priv)
}
}
+static void
+soup_connection_set_state (SoupConnection *conn,
+ SoupConnectionState state)
+{
+ SoupConnectionPrivate *priv = soup_connection_get_instance_private (conn);
+
+ if (priv->state == state)
+ return;
+
+ priv->state = state;
+ if (priv->state == SOUP_CONNECTION_IDLE)
+ start_idle_timer (conn);
+
+ g_object_notify_by_pspec (G_OBJECT (conn), properties[PROP_STATE]);
+}
+
static void
current_msg_got_body (SoupMessage *msg, gpointer user_data)
{
@@ -837,8 +852,7 @@ soup_connection_disconnect (SoupConnection *conn)
priv = soup_connection_get_instance_private (conn);
old_state = priv->state;
- if (old_state != SOUP_CONNECTION_DISCONNECTED)
- soup_connection_set_state (conn, SOUP_CONNECTION_DISCONNECTED);
+ soup_connection_set_state (conn, SOUP_CONNECTION_DISCONNECTED);
if (priv->cancellable) {
g_cancellable_cancel (priv->cancellable);
@@ -933,19 +947,21 @@ soup_connection_is_via_proxy (SoupConnection *conn)
return priv->proxy_uri != NULL;
}
-static gboolean
-is_idle_connection_disconnected (SoupConnection *conn)
+gboolean
+soup_connection_is_idle_open (SoupConnection *conn)
{
- SoupConnectionPrivate *priv = soup_connection_get_instance_private (conn);
+ SoupConnectionPrivate *priv = soup_connection_get_instance_private (conn);
GInputStream *istream;
char buffer[1];
GError *error = NULL;
+ g_assert (priv->state == SOUP_CONNECTION_IDLE);
+
if (!g_socket_is_connected (soup_connection_get_socket (conn)))
- return TRUE;
+ return FALSE;
if (priv->unused_timeout && priv->unused_timeout < time (NULL))
- return TRUE;
+ return FALSE;
istream = g_io_stream_get_input_stream (priv->iostream);
@@ -963,64 +979,48 @@ is_idle_connection_disconnected (SoupConnection *conn)
NULL, &error);
if (!g_error_matches (error, G_IO_ERROR, G_IO_ERROR_WOULD_BLOCK)) {
g_clear_error (&error);
- return TRUE;
+ return FALSE;
}
g_error_free (error);
- return FALSE;
+ return TRUE;
}
SoupConnectionState
soup_connection_get_state (SoupConnection *conn)
{
- SoupConnectionPrivate *priv;
-
- g_return_val_if_fail (SOUP_IS_CONNECTION (conn),
- SOUP_CONNECTION_DISCONNECTED);
- priv = soup_connection_get_instance_private (conn);
-
- if (priv->state == SOUP_CONNECTION_IDLE &&
- is_idle_connection_disconnected (conn))
- soup_connection_set_state (conn, SOUP_CONNECTION_REMOTE_DISCONNECTED);
+ SoupConnectionPrivate *priv = soup_connection_get_instance_private (conn);
return priv->state;
}
void
-soup_connection_set_state (SoupConnection *conn, SoupConnectionState state)
+soup_connection_set_in_use (SoupConnection *conn,
+ gboolean in_use)
{
- SoupConnectionPrivate *priv;
-
- g_return_if_fail (SOUP_IS_CONNECTION (conn));
- g_return_if_fail (state >= SOUP_CONNECTION_NEW &&
- state <= SOUP_CONNECTION_DISCONNECTED);
+ SoupConnectionPrivate *priv = soup_connection_get_instance_private (conn);
- g_object_ref (conn);
- g_object_freeze_notify (G_OBJECT (conn));
+ g_assert (in_use || priv->in_use > 0);
- priv = soup_connection_get_instance_private (conn);
-
- if (priv->current_msg) {
- g_warn_if_fail (state == SOUP_CONNECTION_IDLE ||
- state == SOUP_CONNECTION_DISCONNECTED);
- clear_current_msg (conn);
- }
-
- if (state == SOUP_CONNECTION_IDLE && !priv->reusable) {
- /* This will recursively call set_state() */
- soup_connection_disconnect (conn);
- } else {
- priv->state = state;
+ if (in_use)
+ priv->in_use++;
+ else
+ priv->in_use--;
- if (priv->state == SOUP_CONNECTION_IDLE)
- start_idle_timer (conn);
+ if (priv->in_use > 0) {
+ if (priv->state == SOUP_CONNECTION_IDLE)
+ soup_connection_set_state (conn, SOUP_CONNECTION_IN_USE);
+ return;
+ }
- g_object_notify_by_pspec (G_OBJECT (conn), properties[PROP_STATE]);
- }
+ if (priv->current_msg)
+ clear_current_msg (conn);
- g_object_thaw_notify (G_OBJECT (conn));
- g_object_unref (conn);
+ if (priv->reusable)
+ soup_connection_set_state (conn, SOUP_CONNECTION_IDLE);
+ else
+ soup_connection_disconnect (conn);
}
void
diff --git a/libsoup/soup-connection.h b/libsoup/soup-connection.h
index 9c556029..e0c5ab18 100644
--- a/libsoup/soup-connection.h
+++ b/libsoup/soup-connection.h
@@ -20,7 +20,6 @@ typedef enum {
SOUP_CONNECTION_CONNECTING,
SOUP_CONNECTION_IDLE,
SOUP_CONNECTION_IN_USE,
- SOUP_CONNECTION_REMOTE_DISCONNECTED,
SOUP_CONNECTION_DISCONNECTED
} SoupConnectionState;
@@ -57,9 +56,9 @@ gboolean soup_connection_is_via_proxy (SoupConnection *conn);
gboolean soup_connection_is_tunnelled (SoupConnection *conn);
SoupConnectionState soup_connection_get_state (SoupConnection *conn);
-void soup_connection_set_state (SoupConnection *conn,
- SoupConnectionState state);
-
+void soup_connection_set_in_use (SoupConnection *conn,
+ gboolean in_use);
+gboolean soup_connection_is_idle_open (SoupConnection *conn);
void soup_connection_set_reusable (SoupConnection *conn,
gboolean reusable);
diff --git a/libsoup/soup-message.c b/libsoup/soup-message.c
index ef4d782a..c39522a8 100644
--- a/libsoup/soup-message.c
+++ b/libsoup/soup-message.c
@@ -1421,12 +1421,14 @@ soup_message_set_connection (SoupMessage *msg,
g_signal_handlers_disconnect_by_data (priv->connection, msg);
priv->io_data = NULL;
g_object_remove_weak_pointer (G_OBJECT (priv->connection), (gpointer*)&priv->connection);
+ soup_connection_set_in_use (priv->connection, FALSE);
}
priv->connection = conn;
if (!priv->connection)
return;
+ soup_connection_set_in_use (priv->connection, TRUE);
priv->last_connection_id = soup_connection_get_id (priv->connection);
g_object_add_weak_pointer (G_OBJECT (priv->connection), (gpointer*)&priv->connection);
diff --git a/libsoup/soup-session.c b/libsoup/soup-session.c
index b027a19b..9be57b21 100644
--- a/libsoup/soup-session.c
+++ b/libsoup/soup-session.c
@@ -1296,8 +1296,6 @@ message_restarted (SoupMessage *msg, gpointer user_data)
if (conn &&
(!soup_message_is_keepalive (msg) ||
SOUP_STATUS_IS_REDIRECTION (soup_message_get_status (msg)))) {
- if (soup_connection_get_state (conn) == SOUP_CONNECTION_IN_USE)
- soup_connection_set_state (conn, SOUP_CONNECTION_IDLE);
soup_message_set_connection (item->msg, NULL);
}
@@ -1402,8 +1400,8 @@ soup_session_cleanup_connections (SoupSession *session,
g_hash_table_iter_init (&iter, priv->conns);
while (g_hash_table_iter_next (&iter, &conn, &host)) {
state = soup_connection_get_state (conn);
- if (state == SOUP_CONNECTION_REMOTE_DISCONNECTED ||
- (cleanup_idle && state == SOUP_CONNECTION_IDLE)) {
+ if (state == SOUP_CONNECTION_IDLE &&
+ (cleanup_idle || !soup_connection_is_idle_open (conn))) {
conns = g_slist_prepend (conns, g_object_ref (conn));
g_hash_table_iter_remove (&iter);
drop_connection (session, host, conn);
@@ -1494,7 +1492,7 @@ connection_state_changed (GObject *object, GParamSpec *param, gpointer user_data
SoupSession *session = user_data;
SoupConnection *conn = SOUP_CONNECTION (object);
- if (soup_connection_get_state (conn) == SOUP_CONNECTION_IDLE)
+ if (soup_connection_get_state (conn) == SOUP_CONNECTION_IDLE && soup_connection_is_idle_open (conn))
soup_session_kick_queue (session);
}
@@ -1505,15 +1503,8 @@ soup_session_unqueue_item (SoupSession *session,
SoupSessionPrivate *priv = soup_session_get_instance_private (session);
SoupSessionHost *host;
GSList *f;
- SoupConnection *conn;
- conn = soup_message_get_connection (item->msg);
- if (conn) {
- if (soup_message_get_method (item->msg) != SOUP_METHOD_CONNECT ||
- !SOUP_STATUS_IS_SUCCESSFUL (soup_message_get_status (item->msg)))
- soup_connection_set_state (conn, SOUP_CONNECTION_IDLE);
- soup_message_set_connection (item->msg, NULL);
- }
+ soup_message_set_connection (item->msg, NULL);
if (item->state != SOUP_MESSAGE_FINISHED) {
g_warning ("finished an item with state %d", item->state);
@@ -1744,6 +1735,7 @@ steal_preconnection (SoupSession *session,
if (!preconnect_item->connect_only || preconnect_item->state != SOUP_MESSAGE_CONNECTING)
return FALSE;
+ soup_message_set_connection (item->msg, conn);
soup_message_set_connection (preconnect_item->msg, NULL);
g_assert (preconnect_item->related == NULL);
preconnect_item->related = soup_message_queue_item_ref (item);
@@ -1778,10 +1770,8 @@ get_connection_for_host (SoupSession *session,
switch (soup_connection_get_state (conn)) {
case SOUP_CONNECTION_IDLE:
- if (!need_new_connection) {
- soup_connection_set_state (conn, SOUP_CONNECTION_IN_USE);
+ if (!need_new_connection && soup_connection_is_idle_open (conn))
return conn;
- }
break;
case SOUP_CONNECTION_CONNECTING:
if (steal_preconnection (session, item, conn))
@@ -1902,7 +1892,6 @@ get_connection (SoupMessageQueueItem *item, gboolean *should_cleanup)
case SOUP_CONNECTION_NEW:
break;
case SOUP_CONNECTION_IDLE:
- case SOUP_CONNECTION_REMOTE_DISCONNECTED:
case SOUP_CONNECTION_DISCONNECTED:
g_assert_not_reached ();
}
@@ -2172,8 +2161,7 @@ soup_session_abort (SoupSession *session)
SoupConnectionState state;
state = soup_connection_get_state (conn);
- if (state == SOUP_CONNECTION_IDLE ||
- state == SOUP_CONNECTION_REMOTE_DISCONNECTED) {
+ if (state == SOUP_CONNECTION_IDLE) {
conns = g_slist_prepend (conns, g_object_ref (conn));
g_hash_table_iter_remove (&iter);
drop_connection (session, host, conn);
diff --git a/tests/connection-test.c b/tests/connection-test.c
index cd9c71f1..2e90cce1 100644
--- a/tests/connection-test.c
+++ b/tests/connection-test.c
@@ -701,7 +701,6 @@ connection_state_changed (SoupConnection *conn,
"Unexpected transition: %s -> %s\n",
state_names[*state], state_names[new_state]);
break;
- case SOUP_CONNECTION_REMOTE_DISCONNECTED:
case SOUP_CONNECTION_DISCONNECTED:
soup_test_assert (FALSE,
"Unexpected transition: %s -> %s\n",
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
