Skip to content
This repository was archived by the owner on Apr 28, 2020. It is now read-only.

Commit 276ca2b

Browse files
wjtwjt
authored
Merge pull request #75 from endlessm/T29303-glib-thread-pool-fix2
T29303 Thread pool fix 2
2 parents c97c4ee + 6d0344f commit 276ca2b

11 files changed

+296
-246
lines changed

‎gio/gdbusconnection.c‎

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,6 @@ typedef struct
222222
{
223223
GDestroyNotify callback;
224224
gpointer user_data;
225-
GMainContext *context;
226225
} CallDestroyNotifyData;
227226

228227
static gboolean
@@ -236,8 +235,6 @@ call_destroy_notify_data_in_idle (gpointer user_data)
236235
static void
237236
call_destroy_notify_data_free (CallDestroyNotifyData *data)
238237
{
239-
if (data->context != NULL)
240-
g_main_context_unref (data->context);
241238
g_free (data);
242239
}
243240

@@ -258,14 +255,11 @@ call_destroy_notify (GMainContext *context,
258255
CallDestroyNotifyData *data;
259256

260257
if (callback == NULL)
261-
goto out;
258+
return;
262259

263260
data = g_new0 (CallDestroyNotifyData, 1);
264261
data->callback = callback;
265262
data->user_data = user_data;
266-
data->context = context;
267-
if (data->context != NULL)
268-
g_main_context_ref (data->context);
269263

270264
idle_source = g_idle_source_new ();
271265
g_source_set_priority (idle_source, G_PRIORITY_DEFAULT);
@@ -274,11 +268,8 @@ call_destroy_notify (GMainContext *context,
274268
data,
275269
(GDestroyNotify) call_destroy_notify_data_free);
276270
g_source_set_name (idle_source, "[gio] call_destroy_notify_data_in_idle");
277-
g_source_attach (idle_source, data->context);
271+
g_source_attach (idle_source, context);
278272
g_source_unref (idle_source);
279-
280-
out:
281-
;
282273
}
283274

284275
/* ---------------------------------------------------------------------------------------------------- */
@@ -3703,6 +3694,14 @@ unsubscribe_id_internal (GDBusConnection *connection,
37033694
*
37043695
* Unsubscribes from signals.
37053696
*
3697+
* Note that there may still be D-Bus traffic to process (relating to this
3698+
* signal subscription) in the current thread-default #GMainContext after this
3699+
* function has returned. You should continue to iterate the #GMainContext
3700+
* until the #GDestroyNotify function passed to
3701+
* g_dbus_connection_signal_subscribe() is called, in order to avoid memory
3702+
* leaks through callbacks queued on the #GMainContext after it’s stopped being
3703+
* iterated.
3704+
*
37063705
* Since: 2.26
37073706
*/
37083707
void

‎gio/gdbusnameowning.c‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -877,6 +877,13 @@ g_bus_own_name_on_connection_with_closures (GDBusConnection *connection,
877877
*
878878
* Stops owning a name.
879879
*
880+
* Note that there may still be D-Bus traffic to process (relating to owning
881+
* and unowning the name) in the current thread-default #GMainContext after
882+
* this function has returned. You should continue to iterate the #GMainContext
883+
* until the #GDestroyNotify function passed to g_bus_own_name() is called, in
884+
* order to avoid memory leaks through callbacks queued on the #GMainContext
885+
* after it’s stopped being iterated.
886+
*
880887
* Since: 2.26
881888
*/
882889
void

‎gio/gdbusnamewatching.c‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -858,6 +858,13 @@ guint g_bus_watch_name_on_connection_with_closures (
858858
*
859859
* Stops watching a name.
860860
*
861+
* Note that there may still be D-Bus traffic to process (relating to watching
862+
* and unwatching the name) in the current thread-default #GMainContext after
863+
* this function has returned. You should continue to iterate the #GMainContext
864+
* until the #GDestroyNotify function passed to g_bus_watch_name() is called, in
865+
* order to avoid memory leaks through callbacks queued on the #GMainContext
866+
* after it’s stopped being iterated.
867+
*
861868
* Since: 2.26
862869
*/
863870
void

‎gio/gdbusproxy.c‎

Lines changed: 28 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -95,28 +95,19 @@ G_LOCK_DEFINE_STATIC (properties_lock);
9595

9696
/* ---------------------------------------------------------------------------------------------------- */
9797

98-
G_LOCK_DEFINE_STATIC (signal_subscription_lock);
99-
100-
typedef struct
101-
{
102-
volatile gint ref_count;
103-
GDBusProxy *proxy;
104-
} SignalSubscriptionData;
105-
106-
static SignalSubscriptionData *
107-
signal_subscription_ref (SignalSubscriptionData *data)
98+
static GWeakRef *
99+
weak_ref_new (GObject *object)
108100
{
109-
g_atomic_int_inc (&data->ref_count);
110-
return data;
101+
GWeakRef *weak_ref = g_new0 (GWeakRef, 1);
102+
g_weak_ref_init (weak_ref, object);
103+
return g_steal_pointer (&weak_ref);
111104
}
112105

113106
static void
114-
signal_subscription_unref (SignalSubscriptionData *data)
107+
weak_ref_free (GWeakRef *weak_ref)
115108
{
116-
if (g_atomic_int_dec_and_test (&data->ref_count))
117-
{
118-
g_slice_free (SignalSubscriptionData, data);
119-
}
109+
g_weak_ref_clear (weak_ref);
110+
g_free (weak_ref);
120111
}
121112

122113
/* ---------------------------------------------------------------------------------------------------- */
@@ -152,8 +143,6 @@ struct _GDBusProxyPrivate
152143

153144
/* mutable, protected by properties_lock */
154145
GDBusObject *object;
155-
156-
SignalSubscriptionData *signal_subscription_data;
157146
};
158147

159148
enum
@@ -189,22 +178,6 @@ G_DEFINE_TYPE_WITH_CODE (GDBusProxy, g_dbus_proxy, G_TYPE_OBJECT,
189178
G_IMPLEMENT_INTERFACE (G_TYPE_INITABLE, initable_iface_init)
190179
G_IMPLEMENT_INTERFACE (G_TYPE_ASYNC_INITABLE, async_initable_iface_init))
191180

192-
static void
193-
g_dbus_proxy_dispose (GObject *object)
194-
{
195-
GDBusProxy *proxy = G_DBUS_PROXY (object);
196-
G_LOCK (signal_subscription_lock);
197-
if (proxy->priv->signal_subscription_data != NULL)
198-
{
199-
proxy->priv->signal_subscription_data->proxy = NULL;
200-
signal_subscription_unref (proxy->priv->signal_subscription_data);
201-
proxy->priv->signal_subscription_data = NULL;
202-
}
203-
G_UNLOCK (signal_subscription_lock);
204-
205-
G_OBJECT_CLASS (g_dbus_proxy_parent_class)->dispose (object);
206-
}
207-
208181
static void
209182
g_dbus_proxy_finalize (GObject *object)
210183
{
@@ -346,7 +319,6 @@ g_dbus_proxy_class_init (GDBusProxyClass *klass)
346319
{
347320
GObjectClass *gobject_class = G_OBJECT_CLASS (klass);
348321

349-
gobject_class->dispose = g_dbus_proxy_dispose;
350322
gobject_class->finalize = g_dbus_proxy_finalize;
351323
gobject_class->set_property = g_dbus_proxy_set_property;
352324
gobject_class->get_property = g_dbus_proxy_get_property;
@@ -638,9 +610,6 @@ static void
638610
g_dbus_proxy_init (GDBusProxy *proxy)
639611
{
640612
proxy->priv = g_dbus_proxy_get_instance_private (proxy);
641-
proxy->priv->signal_subscription_data = g_slice_new0 (SignalSubscriptionData);
642-
proxy->priv->signal_subscription_data->ref_count = 1;
643-
proxy->priv->signal_subscription_data->proxy = proxy;
644613
proxy->priv->properties = g_hash_table_new_full (g_str_hash,
645614
g_str_equal,
646615
g_free,
@@ -868,21 +837,12 @@ on_signal_received (GDBusConnection *connection,
868837
GVariant *parameters,
869838
gpointer user_data)
870839
{
871-
SignalSubscriptionData *data = user_data;
840+
GWeakRef *proxy_weak = user_data;
872841
GDBusProxy *proxy;
873842

874-
G_LOCK (signal_subscription_lock);
875-
proxy = data->proxy;
843+
proxy = G_DBUS_PROXY (g_weak_ref_get (proxy_weak));
876844
if (proxy == NULL)
877-
{
878-
G_UNLOCK (signal_subscription_lock);
879-
return;
880-
}
881-
else
882-
{
883-
g_object_ref (proxy);
884-
G_UNLOCK (signal_subscription_lock);
885-
}
845+
return;
886846

887847
if (!proxy->priv->initialized)
888848
goto out;
@@ -929,8 +889,7 @@ on_signal_received (GDBusConnection *connection,
929889
parameters);
930890

931891
out:
932-
if (proxy != NULL)
933-
g_object_unref (proxy);
892+
g_clear_object (&proxy);
934893
}
935894

936895
/* ---------------------------------------------------------------------------------------------------- */
@@ -1038,7 +997,7 @@ on_properties_changed (GDBusConnection *connection,
1038997
GVariant *parameters,
1039998
gpointer user_data)
1040999
{
1041-
SignalSubscriptionData *data = user_data;
1000+
GWeakRef *proxy_weak = user_data;
10421001
gboolean emit_g_signal = FALSE;
10431002
GDBusProxy *proxy;
10441003
const gchar *interface_name_for_signal;
@@ -1052,18 +1011,9 @@ on_properties_changed (GDBusConnection *connection,
10521011
changed_properties = NULL;
10531012
invalidated_properties = NULL;
10541013

1055-
G_LOCK (signal_subscription_lock);
1056-
proxy = data->proxy;
1014+
proxy = G_DBUS_PROXY (g_weak_ref_get (proxy_weak));
10571015
if (proxy == NULL)
1058-
{
1059-
G_UNLOCK (signal_subscription_lock);
1060-
goto out;
1061-
}
1062-
else
1063-
{
1064-
g_object_ref (proxy);
1065-
G_UNLOCK (signal_subscription_lock);
1066-
}
1016+
return;
10671017

10681018
if (!proxy->priv->initialized)
10691019
goto out;
@@ -1150,11 +1100,9 @@ on_properties_changed (GDBusConnection *connection,
11501100
}
11511101

11521102
out:
1153-
if (changed_properties != NULL)
1154-
g_variant_unref (changed_properties);
1103+
g_clear_pointer (&changed_properties, g_variant_unref);
11551104
g_free (invalidated_properties);
1156-
if (proxy != NULL)
1157-
g_object_unref (proxy);
1105+
g_clear_object (&proxy);
11581106
}
11591107

11601108
/* ---------------------------------------------------------------------------------------------------- */
@@ -1258,8 +1206,7 @@ on_name_owner_changed_get_all_cb (GDBusConnection *connection,
12581206
{
12591207
G_LOCK (properties_lock);
12601208
g_free (data->proxy->priv->name_owner);
1261-
data->proxy->priv->name_owner = data->name_owner;
1262-
data->name_owner = NULL; /* to avoid an extra copy, we steal the string */
1209+
data->proxy->priv->name_owner = g_steal_pointer (&data->name_owner);
12631210
g_hash_table_remove_all (data->proxy->priv->properties);
12641211
G_UNLOCK (properties_lock);
12651212
if (result != NULL)
@@ -1289,23 +1236,14 @@ on_name_owner_changed (GDBusConnection *connection,
12891236
GVariant *parameters,
12901237
gpointer user_data)
12911238
{
1292-
SignalSubscriptionData *data = user_data;
1239+
GWeakRef *proxy_weak = user_data;
12931240
GDBusProxy *proxy;
12941241
const gchar *old_owner;
12951242
const gchar *new_owner;
12961243

1297-
G_LOCK (signal_subscription_lock);
1298-
proxy = data->proxy;
1244+
proxy = G_DBUS_PROXY (g_weak_ref_get (proxy_weak));
12991245
if (proxy == NULL)
1300-
{
1301-
G_UNLOCK (signal_subscription_lock);
1302-
goto out;
1303-
}
1304-
else
1305-
{
1306-
g_object_ref (proxy);
1307-
G_UNLOCK (signal_subscription_lock);
1308-
}
1246+
return;
13091247

13101248
/* if we are already trying to load properties, cancel that */
13111249
if (proxy->priv->get_all_cancellable != NULL)
@@ -1415,8 +1353,7 @@ on_name_owner_changed (GDBusConnection *connection,
14151353
}
14161354

14171355
out:
1418-
if (proxy != NULL)
1419-
g_object_unref (proxy);
1356+
g_clear_object (&proxy);
14201357
}
14211358

14221359
/* ---------------------------------------------------------------------------------------------------- */
@@ -1762,8 +1699,8 @@ async_initable_init_first (GAsyncInitable *initable)
17621699
proxy->priv->interface_name,
17631700
G_DBUS_SIGNAL_FLAGS_NONE,
17641701
on_properties_changed,
1765-
signal_subscription_ref (proxy->priv->signal_subscription_data),
1766-
(GDestroyNotify) signal_subscription_unref);
1702+
weak_ref_new (G_OBJECT (proxy)),
1703+
(GDestroyNotify) weak_ref_free);
17671704
}
17681705

17691706
if (!(proxy->priv->flags & G_DBUS_PROXY_FLAGS_DO_NOT_CONNECT_SIGNALS))
@@ -1778,8 +1715,8 @@ async_initable_init_first (GAsyncInitable *initable)
17781715
NULL, /* arg0 */
17791716
G_DBUS_SIGNAL_FLAGS_NONE,
17801717
on_signal_received,
1781-
signal_subscription_ref (proxy->priv->signal_subscription_data),
1782-
(GDestroyNotify) signal_subscription_unref);
1718+
weak_ref_new (G_OBJECT (proxy)),
1719+
(GDestroyNotify) weak_ref_free);
17831720
}
17841721

17851722
if (proxy->priv->name != NULL &&
@@ -1794,8 +1731,8 @@ async_initable_init_first (GAsyncInitable *initable)
17941731
proxy->priv->name, /* arg0 */
17951732
G_DBUS_SIGNAL_FLAGS_NONE,
17961733
on_name_owner_changed,
1797-
signal_subscription_ref (proxy->priv->signal_subscription_data),
1798-
(GDestroyNotify) signal_subscription_unref);
1734+
weak_ref_new (G_OBJECT (proxy)),
1735+
(GDestroyNotify) weak_ref_free);
17991736
}
18001737
}
18011738

‎gio/tests/g-file-info-filesystem-readonly.c‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@
2525
#include <gio/gio.h>
2626
#include <gio/gunixmounts.h>
2727

28-
static void
28+
static gboolean
2929
run (GError **error,
3030
const gchar *argv0,
3131
...)
@@ -34,6 +34,8 @@ run (GError **error,
3434
const gchar *arg;
3535
va_list ap;
3636
GSubprocess *subprocess;
37+
gchar *command_line = NULL;
38+
gboolean success;
3739

3840
args = g_ptr_array_new ();
3941

@@ -44,14 +46,20 @@ run (GError **error,
4446
g_ptr_array_add (args, NULL);
4547
va_end (ap);
4648

49+
command_line = g_strjoinv (" ", (gchar **) args->pdata);
50+
g_test_message ("Running command `%s`", command_line);
51+
g_free (command_line);
52+
4753
subprocess = g_subprocess_newv ((const gchar * const *) args->pdata, G_SUBPROCESS_FLAGS_NONE, error);
4854
g_ptr_array_free (args, TRUE);
4955

5056
if (subprocess == NULL)
51-
return;
57+
return FALSE;
5258

53-
g_subprocess_wait_check (subprocess, NULL, error);
59+
success = g_subprocess_wait_check (subprocess, NULL, error);
5460
g_object_unref (subprocess);
61+
62+
return success;
5563
}
5664

5765
static void
@@ -135,8 +143,14 @@ test_filesystem_readonly (gconstpointer with_mount_monitor)
135143

136144
/* Use bindfs, which does not need root privileges, to mount the contents of one dir
137145
* into another dir (and do the mount as readonly as per passed '-o ro' option) */
138-
run (&error, bindfs, "-n", "-o", "ro", dir_to_mount, dir_mountpoint, NULL);
139-
g_assert_no_error (error);
146+
if (!run (&error, bindfs, "-n", "-o", "ro", dir_to_mount, dir_mountpoint, NULL))
147+
{
148+
gchar *skip_message = g_strdup_printf ("Failed to run bindfs to set up test: %s", error->message);
149+
g_test_skip (skip_message);
150+
g_free (skip_message);
151+
g_clear_error (&error);
152+
return;
153+
}
140154

141155
/* Let's check now, that the file is in indeed in a readonly filesystem */
142156
file_in_mountpoint = g_strdup_printf ("%s/example.txt", dir_mountpoint);

0 commit comments

Comments
 (0)