From 776e479de8f97a2cb9403acd3c570fd010e61ed6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Wed, 6 Dec 2017 13:24:28 -0500 Subject: [PATCH 1/3] object: Show error when using proto functions (connect*, emit) on destroyed object We also need to ensure that these proto functions are used properly. --- gi/object.cpp | 21 +++++++++++++ .../js/testGObjectDestructionAccess.js | 30 +++++++++++++++++++ 2 files changed, 51 insertions(+) diff --git a/gi/object.cpp b/gi/object.cpp index 4ad951fc4..16a23e2be 100644 --- a/gi/object.cpp +++ b/gi/object.cpp @@ -1681,6 +1681,16 @@ real_connect_func(JSContext *context, priv->info ? g_base_info_get_name( (GIBaseInfo*) priv->info) : g_type_name(priv->gtype)); return false; } + if (priv->g_object_finalized) { + g_critical("Object %s.%s (%p), has been already deallocated - impossible to connect to signal. " + "This might be caused by the fact that the object has been destroyed from C " + "code using something such as destroy(), dispose(), or remove() vfuncs", + priv->info ? g_base_info_get_namespace( (GIBaseInfo*) priv->info) : "", + priv->info ? g_base_info_get_name( (GIBaseInfo*) priv->info) : g_type_name(priv->gtype), + priv->gobj); + gjs_dumpstack(); + return true; + } if (argc != 2 || !argv[0].isString() || !JS::IsCallable(&argv[1].toObject())) { gjs_throw(context, "connect() takes two args, the signal name and the callback"); @@ -1768,6 +1778,17 @@ emit_func(JSContext *context, return false; } + if (priv->g_object_finalized) { + g_critical("Object %s.%s (%p), has been already deallocated - impossible to emit signal. " + "This might be caused by the fact that the object has been destroyed from C " + "code using something such as destroy(), dispose(), or remove() vfuncs", + priv->info ? g_base_info_get_namespace( (GIBaseInfo*) priv->info) : "", + priv->info ? g_base_info_get_name( (GIBaseInfo*) priv->info) : g_type_name(priv->gtype), + priv->gobj); + gjs_dumpstack(); + return true; + } + if (argc < 1 || !argv[0].isString()) { gjs_throw(context, "emit() first arg is the signal name"); return false; diff --git a/installed-tests/js/testGObjectDestructionAccess.js b/installed-tests/js/testGObjectDestructionAccess.js index 3e30e6e48..22c01b0f0 100644 --- a/installed-tests/js/testGObjectDestructionAccess.js +++ b/installed-tests/js/testGObjectDestructionAccess.js @@ -55,4 +55,34 @@ describe('Access to destroyed GObject', () => { GLib.test_assert_expected_messages_internal('Gjs', 'testGObjectDestructionAccess.js', 0, 'testExceptionInDestroyedObjectMethodSet'); }); + + it('Proto function connect', () => { + GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_CRITICAL, + 'Object Gtk.Window (0x*'); + + destroyedWindow.connect('foo-signal', () => {}); + + GLib.test_assert_expected_messages_internal('Gjs', 'testGObjectDestructionAccess.js', 0, + 'testExceptionInDestroyedObjectConnect'); + }); + + it('Proto function connect_after', () => { + GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_CRITICAL, + 'Object Gtk.Window (0x*'); + + destroyedWindow.connect_after('foo-signal', () => {}); + + GLib.test_assert_expected_messages_internal('Gjs', 'testGObjectDestructionAccess.js', 0, + 'testExceptionInDestroyedObjectConnectAfter'); + }); + + it('Proto function emit', () => { + GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_CRITICAL, + 'Object Gtk.Window (0x*'); + + destroyedWindow.emit('foo-signal'); + + GLib.test_assert_expected_messages_internal('Gjs', 'testGObjectDestructionAccess.js', 0, + 'testExceptionInDestroyedObjectEmit'); + }); }); -- GitLab From 54e1703eb8044f9739bed97a5bdb9619aaf84a25 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Wed, 6 Dec 2017 13:26:50 -0500 Subject: [PATCH 2/3] object: make clear in toString() proto method when the object is finalized --- gi/object.cpp | 4 +++- installed-tests/js/testGObjectDestructionAccess.js | 4 ++++ 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/gi/object.cpp b/gi/object.cpp index 16a23e2be..fa117c1ad 100644 --- a/gi/object.cpp +++ b/gi/object.cpp @@ -1877,7 +1877,9 @@ to_string_func(JSContext *context, return false; /* wrong class passed in */ } - return _gjs_proxy_to_string_func(context, obj, "object", + return _gjs_proxy_to_string_func(context, obj, + (priv->g_object_finalized) ? + "object (FINALIZED)" : "object", (GIBaseInfo*)priv->info, priv->gtype, priv->gobj, rec.rval()); } diff --git a/installed-tests/js/testGObjectDestructionAccess.js b/installed-tests/js/testGObjectDestructionAccess.js index 22c01b0f0..c4eb307dc 100644 --- a/installed-tests/js/testGObjectDestructionAccess.js +++ b/installed-tests/js/testGObjectDestructionAccess.js @@ -85,4 +85,8 @@ describe('Access to destroyed GObject', () => { GLib.test_assert_expected_messages_internal('Gjs', 'testGObjectDestructionAccess.js', 0, 'testExceptionInDestroyedObjectEmit'); }); + + it('Proto function toString', () => { + expect(destroyedWindow.toString()).toMatch(/\[object \(FINALIZED\) instance proxy GIName:Gtk.Window jsobj@0x[a-f0-9]+ native@0x[a-f0-9]+\]/); + }); }); -- GitLab From 0b3d6bb434a13389a5de48a2d849e6a13bdb084f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= Date: Thu, 28 Dec 2017 19:47:27 +0100 Subject: [PATCH 3/3] object: only reset keep-alive and disassociate if needed Fixes a crash in testGDBus.js --- gi/object.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/gi/object.cpp b/gi/object.cpp index fa117c1ad..b6d6557a2 100644 --- a/gi/object.cpp +++ b/gi/object.cpp @@ -974,8 +974,11 @@ wrapped_gobj_dispose_notify(gpointer data, priv->g_object_finalized = true; - priv->keep_alive.reset(); - dissociate_list_remove(priv); + if (priv->keep_alive.rooted()) { + priv->keep_alive.reset(); + dissociate_list_remove(priv); + } + weak_pointer_list.erase(priv); #if DEBUG_DISPOSE gjs_debug(GJS_DEBUG_GOBJECT, "Wrapped GObject %p disposed", where_the_object_was); -- GitLab