From 27a073f1a65c60e0568a5b379a6eaad385c717b0 Mon Sep 17 00:00:00 2001 From: Colomban Wendling Date: Fri, 11 Apr 2014 22:56:30 +0200 Subject: [PATCH 1/2] Make plugin_signal_connect() safe on any object Watch the lifetime of objects referenced in plugin->signal_ids and remove our references to them if they get destroyed. This avoids possibly trying to disconnect signals on destroyed objects when the plugin is unloaded. Supporting this case is safer, and is useful for objects that may or may not outlive the plugin (like ScintillaObjects), because in such cases plugin_signal_connect() is handy to make sure the signals are disconnected if the object is still alive, but used to crash if the object was destroyed. --- src/plugindata.h | 2 +- src/pluginprivate.h | 3 +++ src/plugins.c | 34 ++++++++++++++++++++++++++++++++++ src/pluginutils.c | 11 +++++++++-- 4 files changed, 47 insertions(+), 3 deletions(-) diff --git a/src/plugindata.h b/src/plugindata.h index d7e681fd..f766b9bf 100644 --- a/src/plugindata.h +++ b/src/plugindata.h @@ -55,7 +55,7 @@ G_BEGIN_DECLS * @warning You should not test for values below 200 as previously * @c GEANY_API_VERSION was defined as an enum value, not a macro. */ -#define GEANY_API_VERSION 217 +#define GEANY_API_VERSION 218 /* hack to have a different ABI when built with GTK3 because loading GTK2-linked plugins * with GTK3-linked Geany leads to crash */ diff --git a/src/pluginprivate.h b/src/pluginprivate.h index 63ebdd23..aa6a5efc 100644 --- a/src/pluginprivate.h +++ b/src/pluginprivate.h @@ -61,4 +61,7 @@ GeanyPluginPrivate; typedef GeanyPluginPrivate Plugin; /* shorter alias */ +void plugin_watch_object(Plugin *plugin, gpointer object); + + #endif /* GEANY_PLUGINPRIVATE_H */ diff --git a/src/plugins.c b/src/plugins.c index 08523d89..ae60bf77 100644 --- a/src/plugins.c +++ b/src/plugins.c @@ -769,6 +769,37 @@ plugin_new(const gchar *fname, gboolean init_plugin, gboolean add_to_list) } +static void on_object_weak_notify(gpointer data, GObject *old_ptr) +{ + Plugin *plugin = data; + guint i = 0; + + g_return_if_fail(plugin && plugin->signal_ids); + + for (i = 0; i < plugin->signal_ids->len; i++) + { + SignalConnection *sc = &g_array_index(plugin->signal_ids, SignalConnection, i); + + if (sc->object == old_ptr) + { + g_array_remove_index_fast(plugin->signal_ids, i); + /* we can break the loop right after finding the first match, + * because we will get one notification per connected signal */ + break; + } + } +} + + +/* add an object to watch for destruction, and release pointers to it when destroyed. + * this should only be used by plugin_signal_connect() to add a watch on + * the object lifetime and nuke out references to it in plugin->signal_ids */ +void plugin_watch_object(Plugin *plugin, gpointer object) +{ + g_object_weak_ref(object, on_object_weak_notify, plugin); +} + + static void remove_callbacks(Plugin *plugin) { GArray *signal_ids = plugin->signal_ids; @@ -778,7 +809,10 @@ static void remove_callbacks(Plugin *plugin) return; foreach_array(SignalConnection, sc, signal_ids) + { g_signal_handler_disconnect(sc->object, sc->handler_id); + g_object_weak_unref(sc->object, on_object_weak_notify, plugin); + } g_array_free(signal_ids, TRUE); } diff --git a/src/pluginutils.c b/src/pluginutils.c index e8a50dae..7c2660ad 100644 --- a/src/pluginutils.c +++ b/src/pluginutils.c @@ -105,7 +105,8 @@ void plugin_module_make_resident(GeanyPlugin *plugin) * @param user_data The user data passed to the signal handler. * @see plugin_callbacks. * - * @warning This should only be used on objects that outlive the plugin, never on + * @warning Before version 1.25 (API < 218), + * this should only be used on objects that outlive the plugin, never on * objects that will get destroyed before the plugin is unloaded. For objects * created and destroyed by the plugin, you can simply use @c g_signal_connect(), * since all handlers are disconnected when the object is destroyed anyway. @@ -116,7 +117,10 @@ void plugin_module_make_resident(GeanyPlugin *plugin) * disconnect the signal on @c plugin_cleanup()), and when the object is destroyed * during the plugin's lifetime (in which case you cannot and should not disconnect * manually in @c plugin_cleanup() since it already has been disconnected and the - * object has been destroyed), and disconnect yourself or not as appropriate. */ + * object has been destroyed), and disconnect yourself or not as appropriate. + * @note Since version 1.25 (API >= 218), the object lifetime is watched and so the above + * restriction does not apply. However, for objects destroyed by the plugin, + * @c g_signal_connect() is safe and has lower overhead. */ void plugin_signal_connect(GeanyPlugin *plugin, GObject *object, const gchar *signal_name, gboolean after, GCallback callback, gpointer user_data) @@ -137,6 +141,9 @@ void plugin_signal_connect(GeanyPlugin *plugin, sc.object = object; sc.handler_id = id; g_array_append_val(plugin->priv->signal_ids, sc); + + /* watch the object lifetime to nuke our pointers to it */ + plugin_watch_object(plugin->priv, object); } From 75542c6d3c51663a0c23243a8c7cf2555fa12858 Mon Sep 17 00:00:00 2001 From: Colomban Wendling Date: Fri, 11 Apr 2014 23:04:14 +0200 Subject: [PATCH 2/2] Add defensive checks on plugin_signal_connect()'s sensitive arguments --- src/pluginutils.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/pluginutils.c b/src/pluginutils.c index 7c2660ad..e8c91c50 100644 --- a/src/pluginutils.c +++ b/src/pluginutils.c @@ -128,6 +128,9 @@ void plugin_signal_connect(GeanyPlugin *plugin, gulong id; SignalConnection sc; + g_return_if_fail(plugin != NULL); + g_return_if_fail(object == NULL || G_IS_OBJECT(object)); + if (!object) object = geany_object;