luc14n0/glib
1
0
Fork 0
mirror of https://gitlab.gnome.org/GNOME/glib.git synced 2024-11-09 19:06:15 +01:00
Code Issues Packages Projects Releases Wiki Activity

gdbusconnection: Rearrange refcount handling of map_method_serial_to_task

Browse Source 
It already implicitly held a strong ref on its `GTask` values, but
didn’t have a free function set so that they would be automatically
unreffed on removal from the map.

This meant that the functions handling removals from the map,
`on_worker_closed()` (via `cancel_method_on_close()`) and
`send_message_with_reply_cleanup()` had to call unref once more than
they would otherwise.

In `send_message_with_reply_cleanup()`, this behaviour depended on
whether it was called with `remove == TRUE`. If not, it was `(transfer
none)` not `(transfer full)`. This led to bugs in its callers.

For example, this led to a direct leak in `cancel_method_on_close()`, as
it needed to remove tasks from `map_method_serial_to_task`, but called
`send_message_with_reply_cleanup(remove = FALSE)` and erroneously didn’t
call unref an additional time.

Try and simplify it all by setting a `GDestroyNotify` on
`map_method_serial_to_task`’s values, and making the refcount handling
of `send_message_with_reply_cleanup()` not be conditional on its
arguments.

Signed-off-by: Philip Withnall <pwithnall@endlessos.org>

Helps: #1264
This commit is contained in:
Philip Withnall 2023-02-22 12:19:16 +00:00
parent 08a4387678
commit b84ec21f9c
1 changed files with 12 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

24
gio/gdbusconnection.c
View File

@ -409,7 +409,7 @@ struct _GDBusConnection
GDBusConnectionFlags flags;
/* Map used for managing method replies, protected by @lock */
GHashTable *map_method_serial_to_task; /* guint32 -> GTask* */
GHashTable *map_method_serial_to_task; /* guint32 -> owned GTask* */
/* Maps used for managing signal subscription, protected by @lock */
GHashTable *map_rule_to_signal_data; /* match rule (gchar*) -> SignalData */
@ -1061,7 +1061,7 @@ g_dbus_connection_init (GDBusConnection *connection)
g_mutex_init (&connection->lock);
g_mutex_init (&connection->init_lock);
connection->map_method_serial_to_task = g_hash_table_new (g_direct_hash, g_direct_equal);
connection->map_method_serial_to_task = g_hash_table_new_full (g_direct_hash, g_direct_equal, NULL, g_object_unref);
connection->map_rule_to_signal_data = g_hash_table_new (g_str_hash,
g_str_equal);
@ -1768,7 +1768,7 @@ send_message_data_free (SendMessageData *data)
/* ---------------------------------------------------------------------------------------------------- */
/* can be called from any thread with lock held; @task is (transfer full) */
/* can be called from any thread with lock held; @task is (transfer none) */
static void
send_message_with_reply_cleanup (GTask *task, gboolean remove)
{
@ -1798,13 +1798,11 @@ send_message_with_reply_cleanup (GTask *task, gboolean remove)
GUINT_TO_POINTER (data->serial));
g_warn_if_fail (removed);
}
g_object_unref (task);
}
/* ---------------------------------------------------------------------------------------------------- */
/* Called from GDBus worker thread with lock held; @task is (transfer full). */
/* Called from GDBus worker thread with lock held; @task is (transfer none). */
static void
send_message_data_deliver_reply_unlocked (GTask *task,
GDBusMessage *reply)
@ -1822,7 +1820,7 @@ send_message_data_deliver_reply_unlocked (GTask *task,
;
}
/* Called from a user thread, lock is not held; @task is (transfer full) */
/* Called from a user thread, lock is not held; @task is (transfer none) */
static void
send_message_data_deliver_error (GTask *task,
GQuark domain,
@ -1839,7 +1837,10 @@ send_message_data_deliver_error (GTask *task,
return;
}
/* Hold a ref on @task as send_message_with_reply_cleanup() will remove it
* from the task map and could end up dropping the last reference */
g_object_ref (task);
send_message_with_reply_cleanup (task, TRUE);
CONNECTION_UNLOCK (connection);
@ -1855,8 +1856,7 @@ send_message_with_reply_cancelled_idle_cb (gpointer user_data)
{
GTask *task = user_data;
g_object_ref (task);
send_message_data_deliver_error (g_steal_pointer (&task), G_IO_ERROR, G_IO_ERROR_CANCELLED,
send_message_data_deliver_error (task, G_IO_ERROR, G_IO_ERROR_CANCELLED,
_("Operation was cancelled"));
return FALSE;
}
@ -1886,8 +1886,7 @@ send_message_with_reply_timeout_cb (gpointer user_data)
{
GTask *task = user_data;
g_object_ref (task);
send_message_data_deliver_error (g_steal_pointer (&task), G_IO_ERROR, G_IO_ERROR_TIMED_OUT,
send_message_data_deliver_error (task, G_IO_ERROR, G_IO_ERROR_TIMED_OUT,
_("Timeout was reached"));
return FALSE;
}
@ -2391,7 +2390,8 @@ on_worker_message_about_to_be_sent (GDBusWorker *worker,
return message;
}
/* called with connection lock held, in GDBusWorker thread */
/* called with connection lock held, in GDBusWorker thread
* @key, @value and @user_data are (transfer none) */
static gboolean
cancel_method_on_close (gpointer key, gpointer value, gpointer user_data)
{