GNOME.org
 

[gjs/3v1n0/block-access-only-if-finalized] object: Block access to object only if it is finalized


commit 7c8b878d8c9f406c538167336d30365ec72319bf
Author: Marco Trevisan (Treviño) <mail 3v1n0 net>
Date:   Thu Apr 22 23:10:19 2021 +0200

    object: Block access to object only if it is finalized
    
    For some time now we're warning whether a disposed object is used and
    this is sane, however the data it holds may be still valid and in
    general all the C code we access to is meant to be used after
    disposition. While such code has not to be called when the object is
    finalized.
    
    Since now we properly track objects finalization we can go back to allow
    most of the object actions (all but the signals connections, these
    involve having toggle notifications that are not a tool we can use
    at this point) with the actual object pointer.
    
    In case the object is finalized instead we behave exactly as before.
    
    Adapted tests and added ones for finalization case

 gi/object.cpp                                      |  12 +-
 gi/object.h                                        |   3 +
 installed-tests/js/testGObjectDestructionAccess.js | 202 +++++++++++++++++++--
 installed-tests/js/testGtk3.js                     |   4 +-
 4 files changed, 196 insertions(+), 25 deletions(-)
---
diff --git a/gi/object.cpp b/gi/object.cpp
index e253b843..4217fe43 100644
--- a/gi/object.cpp
+++ b/gi/object.cpp
@@ -193,13 +193,14 @@ bool ObjectInstance::check_gobject_disposed(const char* for_what) const {
         return true;
 
     g_critical(
-        "Object %s.%s (%p), has been already deallocated — impossible to %s "
+        "Object %s.%s (%p), has been already %s — impossible to %s "
         "it. This might be caused by the object having been destroyed from C "
         "code using something such as destroy(), dispose(), or remove() "
         "vfuncs.",
-        ns(), name(), m_ptr.get(), for_what);
+        ns(), name(), m_ptr.get(), m_gobj_finalized ? "finalized" : "disposed",
+        for_what);
     gjs_dumpstack();
-    return false;
+    return !m_gobj_finalized;
 }
 
 ObjectInstance *
@@ -1542,7 +1543,7 @@ bool ObjectInstance::ensure_uses_toggle_ref(JSContext* cx) {
     if (m_uses_toggle_ref)
         return true;
 
-    if (!check_gobject_disposed("add toggle reference on"))
+    if (!check_gobject_disposed("add toggle reference on") || m_gobj_disposed)
         return false;
 
     debug_lifecycle("Switching object instance to toggle ref");
@@ -1983,7 +1984,8 @@ ObjectInstance::connect_impl(JSContext          *context,
 
     gjs_debug_gsignal("connect obj %p priv %p", m_wrapper.get(), this);
 
-    if (!check_gobject_disposed("connect to any signal on")) {
+    if (!check_gobject_disposed("connect to any signal on") ||
+        m_gobj_disposed) {
         args.rval().setInt32(0);
         return true;
     }
diff --git a/gi/object.h b/gi/object.h
index fd04caf6..149d1b49 100644
--- a/gi/object.h
+++ b/gi/object.h
@@ -384,6 +384,9 @@ class ObjectInstance : public GIWrapperInstance<ObjectBase, ObjectPrototype,
     void check_js_object_finalized(void);
     bool ensure_uses_toggle_ref(JSContext* cx);
     [[nodiscard]] bool check_gobject_disposed(const char* for_what) const;
+    [[nodiscard]] bool check_gobject_finalized() const {
+        return !m_gobj_finalized;
+    }
     GJS_JSAPI_RETURN_CONVENTION
     bool signal_match_arguments_from_object(JSContext* cx,
                                             JS::HandleObject props_obj,
diff --git a/installed-tests/js/testGObjectDestructionAccess.js 
b/installed-tests/js/testGObjectDestructionAccess.js
index 1ec25c14..ca647ac4 100644
--- a/installed-tests/js/testGObjectDestructionAccess.js
+++ b/installed-tests/js/testGObjectDestructionAccess.js
@@ -16,14 +16,15 @@ describe('Access to destroyed GObject', function () {
 
     beforeEach(function () {
         destroyedWindow = new Gtk.Window({type: Gtk.WindowType.TOPLEVEL});
+        destroyedWindow.set_title('To be destroyed');
         destroyedWindow.destroy();
     });
 
     it('Get property', function () {
         GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_CRITICAL,
-            'Object Gtk.Window (0x*');
+            'Object Gtk.Window (0x* disposed *');
 
-        expect(destroyedWindow.title).toBeUndefined();
+        expect(destroyedWindow.title).toBe('To be destroyed');
 
         GLib.test_assert_expected_messages_internal('Gjs', 'testGObjectDestructionAccess.js', 0,
             'testExceptionInDestroyedObjectPropertyGet');
@@ -31,9 +32,12 @@ describe('Access to destroyed GObject', function () {
 
     it('Set property', function () {
         GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_CRITICAL,
-            'Object Gtk.Window (0x*');
+            'Object Gtk.Window (0x* disposed *');
+        GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_CRITICAL,
+            'Object Gtk.Window (0x* disposed *');
 
         destroyedWindow.title = 'I am dead';
+        expect(destroyedWindow.title).toBe('I am dead');
 
         GLib.test_assert_expected_messages_internal('Gjs', 'testGObjectDestructionAccess.js', 0,
             'testExceptionInDestroyedObjectPropertySet');
@@ -41,7 +45,7 @@ describe('Access to destroyed GObject', function () {
 
     it('Add expando property', function () {
         GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_CRITICAL,
-            'Object Gtk.Window (0x*');
+            'Object Gtk.Window (0x* disposed *');
 
         destroyedWindow.expandoProperty = 'Hello!';
 
@@ -63,11 +67,9 @@ describe('Access to destroyed GObject', function () {
 
     it('Access to getter method', function () {
         GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_CRITICAL,
-            'Object Gtk.Window (0x*');
-        GLib.test_expect_message('Gtk', GLib.LogLevelFlags.LEVEL_CRITICAL,
-            '*GTK_IS_WINDOW*');
+            'Object Gtk.Window (0x* disposed *');
 
-        expect(destroyedWindow.get_title()).toBeNull();
+        expect(destroyedWindow.get_title()).toBe('To be destroyed');
 
         GLib.test_assert_expected_messages_internal('Gjs', 'testGObjectDestructionAccess.js', 0,
             'testExceptionInDestroyedObjectMethodGet');
@@ -75,11 +77,12 @@ describe('Access to destroyed GObject', function () {
 
     it('Access to setter method', function () {
         GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_CRITICAL,
-            'Object Gtk.Window (0x*');
-        GLib.test_expect_message('Gtk', GLib.LogLevelFlags.LEVEL_CRITICAL,
-            '*GTK_IS_WINDOW*');
+            'Object Gtk.Window (0x* disposed *');
+        GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_CRITICAL,
+            'Object Gtk.Window (0x* disposed *');
 
         destroyedWindow.set_title('I am dead');
+        expect(destroyedWindow.get_title()).toBe('I am dead');
 
         GLib.test_assert_expected_messages_internal('Gjs', 'testGObjectDestructionAccess.js', 0,
             'testExceptionInDestroyedObjectMethodSet');
@@ -87,7 +90,7 @@ describe('Access to destroyed GObject', function () {
 
     it('Proto function connect', function () {
         GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_CRITICAL,
-            'Object Gtk.Window (0x*');
+            'Object Gtk.Window (0x* disposed *');
 
         expect(destroyedWindow.connect('foo-signal', () => {})).toBe(0);
 
@@ -97,7 +100,7 @@ describe('Access to destroyed GObject', function () {
 
     it('Proto function connect_after', function () {
         GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_CRITICAL,
-            'Object Gtk.Window (0x*');
+            'Object Gtk.Window (0x* disposed *');
 
         expect(destroyedWindow.connect_after('foo-signal', () => {})).toBe(0);
 
@@ -107,9 +110,9 @@ describe('Access to destroyed GObject', function () {
 
     it('Proto function emit', function () {
         GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_CRITICAL,
-            'Object Gtk.Window (0x*');
+            'Object Gtk.Window (0x* disposed *');
 
-        expect(destroyedWindow.emit('foo-signal')).toBeUndefined();
+        expect(destroyedWindow.emit('keys-changed')).toBeUndefined();
 
         GLib.test_assert_expected_messages_internal('Gjs', 'testGObjectDestructionAccess.js', 0,
             'testExceptionInDestroyedObjectEmit');
@@ -117,7 +120,7 @@ describe('Access to destroyed GObject', function () {
 
     it('Proto function signals_disconnect', function () {
         GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_CRITICAL,
-            'Object Gtk.Window (0x*');
+            'Object Gtk.Window (0x* disposed *');
 
         expect(GObject.signal_handlers_disconnect_by_func(destroyedWindow, () => {})).toBe(0);
 
@@ -127,7 +130,7 @@ describe('Access to destroyed GObject', function () {
 
     it('Proto function signals_block', function () {
         GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_CRITICAL,
-            'Object Gtk.Window (0x*');
+            'Object Gtk.Window (0x* disposed *');
 
         expect(GObject.signal_handlers_block_by_func(destroyedWindow, () => {})).toBe(0);
 
@@ -137,7 +140,7 @@ describe('Access to destroyed GObject', function () {
 
     it('Proto function signals_unblock', function () {
         GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_CRITICAL,
-            'Object Gtk.Window (0x*');
+            'Object Gtk.Window (0x* disposed *');
 
         expect(GObject.signal_handlers_unblock_by_func(destroyedWindow, () => {})).toBe(0);
 
@@ -163,6 +166,169 @@ describe('Access to destroyed GObject', function () {
     });
 });
 
+describe('Access to finalized GObject', function () {
+    let destroyedWindow;
+
+    beforeAll(function () {
+        Gtk.init(null);
+    });
+
+    beforeEach(function () {
+        destroyedWindow = new Gtk.Window({type: Gtk.WindowType.TOPLEVEL});
+        destroyedWindow.set_title('To be destroyed');
+        destroyedWindow.destroy();
+
+        GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_CRITICAL,
+            'Object Gtk.Window (0x* disposed *');
+        destroyedWindow.unref();
+        GLib.test_assert_expected_messages_internal('Gjs', 'testGObjectDestructionAccess.js', 0,
+            'testExceptionInDestroyedObjectPropertyGet');
+    });
+
+    afterEach(function () {
+        destroyedWindow = null;
+        GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_CRITICAL,
+            '*Object 0x* has been finalized *');
+        System.gc();
+        GLib.test_assert_expected_messages_internal('Gjs', 'testGObjectDestructionAccess.js', 0,
+            'generates a warn on object garbage collection');
+    });
+
+    it('Get property', function () {
+        GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_CRITICAL,
+            'Object Gtk.Window (0x* finalized *');
+
+        expect(destroyedWindow.title).toBeUndefined();
+
+        GLib.test_assert_expected_messages_internal('Gjs', 'testGObjectDestructionAccess.js', 0,
+            'testExceptionInDestroyedObjectPropertyGet');
+    });
+
+    it('Set property', function () {
+        GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_CRITICAL,
+            'Object Gtk.Window (0x* finalized *');
+        GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_CRITICAL,
+            'Object Gtk.Window (0x* finalized *');
+
+        destroyedWindow.title = 'I am dead';
+        expect(destroyedWindow.title).toBeUndefined();
+
+        GLib.test_assert_expected_messages_internal('Gjs', 'testGObjectDestructionAccess.js', 0,
+            'testExceptionInDestroyedObjectPropertySet');
+    });
+
+    it('Add expando property', function () {
+        GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_CRITICAL,
+            'Object Gtk.Window (0x* finalized *');
+
+        destroyedWindow.expandoProperty = 'Hello!';
+
+        GLib.test_assert_expected_messages_internal('Gjs', 'testGObjectDestructionAccess.js', 0,
+            'testExceptionInDestroyedObjectExpandoPropertySet');
+    });
+
+    it('Access to unset expando property', function () {
+        expect(destroyedWindow.expandoProperty).toBeUndefined();
+    });
+
+    it('Access previously set expando property', function () {
+        destroyedWindow = new Gtk.Window({type: Gtk.WindowType.TOPLEVEL});
+        destroyedWindow.expandoProperty = 'Hello!';
+        destroyedWindow.destroy();
+
+        expect(destroyedWindow.expandoProperty).toBe('Hello!');
+    });
+
+    it('Access to getter method', function () {
+        GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_CRITICAL,
+            'Object Gtk.Window (0x* finalized *');
+        GLib.test_expect_message('Gtk', GLib.LogLevelFlags.LEVEL_CRITICAL,
+            '*GTK_IS_WINDOW*');
+
+        expect(destroyedWindow.get_title()).toBeNull();
+
+        GLib.test_assert_expected_messages_internal('Gjs', 'testGObjectDestructionAccess.js', 0,
+            'testExceptionInDestroyedObjectMethodGet');
+    });
+
+    it('Access to setter method', function () {
+        GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_CRITICAL,
+            'Object Gtk.Window (0x* finalized *');
+        GLib.test_expect_message('Gtk', GLib.LogLevelFlags.LEVEL_CRITICAL,
+            '*GTK_IS_WINDOW*');
+
+        destroyedWindow.set_title('I am dead');
+
+        GLib.test_assert_expected_messages_internal('Gjs', 'testGObjectDestructionAccess.js', 0,
+            'testExceptionInDestroyedObjectMethodSet');
+    });
+
+    it('Proto function connect', function () {
+        GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_CRITICAL,
+            'Object Gtk.Window (0x* finalized *');
+
+        expect(destroyedWindow.connect('foo-signal', () => { })).toBe(0);
+
+        GLib.test_assert_expected_messages_internal('Gjs', 'testGObjectDestructionAccess.js', 0,
+            'testExceptionInDestroyedObjectConnect');
+    });
+
+    it('Proto function connect_after', function () {
+        GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_CRITICAL,
+            'Object Gtk.Window (0x* finalized *');
+
+        expect(destroyedWindow.connect_after('foo-signal', () => { })).toBe(0);
+
+        GLib.test_assert_expected_messages_internal('Gjs', 'testGObjectDestructionAccess.js', 0,
+            'testExceptionInDestroyedObjectConnectAfter');
+    });
+
+    it('Proto function emit', function () {
+        GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_CRITICAL,
+            'Object Gtk.Window (0x* finalized *');
+
+        expect(destroyedWindow.emit('keys-changed')).toBeUndefined();
+
+        GLib.test_assert_expected_messages_internal('Gjs', 'testGObjectDestructionAccess.js', 0,
+            'testExceptionInDestroyedObjectEmit');
+    });
+
+    it('Proto function signals_disconnect', function () {
+        GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_CRITICAL,
+            'Object Gtk.Window (0x* finalized *');
+
+        expect(GObject.signal_handlers_disconnect_by_func(destroyedWindow, () => { })).toBe(0);
+
+        GLib.test_assert_expected_messages_internal('Gjs', 'testGObjectDestructionAccess.js', 0,
+            'testExceptionInDestroyedObjectSignalsDisconnect');
+    });
+
+    it('Proto function signals_block', function () {
+        GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_CRITICAL,
+            'Object Gtk.Window (0x* finalized *');
+
+        expect(GObject.signal_handlers_block_by_func(destroyedWindow, () => { })).toBe(0);
+
+        GLib.test_assert_expected_messages_internal('Gjs', 'testGObjectDestructionAccess.js', 0,
+            'testExceptionInDestroyedObjectSignalsBlock');
+    });
+
+    it('Proto function signals_unblock', function () {
+        GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_CRITICAL,
+            'Object Gtk.Window (0x* finalized *');
+
+        expect(GObject.signal_handlers_unblock_by_func(destroyedWindow, () => { })).toBe(0);
+
+        GLib.test_assert_expected_messages_internal('Gjs', 'testGObjectDestructionAccess.js', 0,
+            'testExceptionInDestroyedObjectSignalsUnblock');
+    });
+
+    it('Proto function toString', function () {
+        expect(destroyedWindow.toString()).toMatch(
+            /\[object \(FINALIZED\) instance wrapper GIName:Gtk.Window jsobj@0x[a-f0-9]+ 
native@0x[a-f0-9]+\]/);
+    });
+});
+
 describe('Disposed or finalized GObject', function () {
     beforeAll(function () {
         GjsTestTools.init();
diff --git a/installed-tests/js/testGtk3.js b/installed-tests/js/testGtk3.js
index a6ba2642..73f3ecb4 100644
--- a/installed-tests/js/testGtk3.js
+++ b/installed-tests/js/testGtk3.js
@@ -225,8 +225,8 @@ describe('Gtk overrides', function () {
         expect(handleDispose).toHaveBeenCalledWith(label);
 
         GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_CRITICAL,
-            'Object Gtk.Label (0x* deallocated *');
-        expect(label.label).toBeUndefined();
+            'Object Gtk.Label (0x* disposed *');
+        expect(label.label).toBe('Hello');
         GLib.test_assert_expected_messages_internal('Gjs', 'testGtk3.js', 0,
             'GTK destroy signal is emitted while disposing objects');
     });
[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]