gobject: Guard against null JS wrapper in set/get property

The wrapper object may be disassociated from the GObject if dispose has
been run. In that case, the pointers in the get/set property vfuncs may
be null. Handle that case with a warning and don't get or set the
property.

Closes: #510

2 files changed, 47 insertions, 0 deletions

diff --git a/gi/gobject.cpp b/gi/gobject.cpp
index 914f6176..809e24cf 100644
--- a/gi/gobject.cpp
+++ b/gi/gobject.cpp
@@ -203,6 +203,12 @@ static void gjs_object_set_gproperty(GObject* object,
                                      unsigned property_id [[maybe_unused]],
                                      const GValue* value, GParamSpec* pspec) {
     auto* priv = ObjectInstance::for_gobject(object);
+    if (!priv) {
+        g_warning("Wrapper for GObject %p was disposed, cannot set property %s",
+                  object, g_param_spec_get_name(pspec));
+        return;
+    }
+
     JSContext* cx = current_js_context();
 
     JS::RootedObject js_obj(cx, priv->wrapper());
@@ -216,6 +222,12 @@ static void gjs_object_get_gproperty(GObject* object,
                                      unsigned property_id [[maybe_unused]],
                                      GValue* value, GParamSpec* pspec) {
     auto* priv = ObjectInstance::for_gobject(object);
+    if (!priv) {
+        g_warning("Wrapper for GObject %p was disposed, cannot get property %s",
+                  object, g_param_spec_get_name(pspec));
+        return;
+    }
+
     JSContext* cx = current_js_context();
 
     JS::RootedObject js_obj(cx, priv->wrapper());
diff --git a/installed-tests/js/testIntrospection.js b/installed-tests/js/testIntrospection.js
index 1037b50a..bdd4ddd4 100644
--- a/installed-tests/js/testIntrospection.js
+++ b/installed-tests/js/testIntrospection.js
@@ -140,6 +140,41 @@ describe('Garbage collection of introspected objects', function () {
         System.gc();
         GLib.idle_add(GLib.PRIORITY_LOW, () => done());
     });
+
+    // This tests a race condition that would crash; it should warn instead
+    it('handles setting a property from C on an object whose JS wrapper has been collected', function (done) {
+        class SomeObject extends GObject.Object {
+            static [GObject.properties] = {
+                'screenfull': GObject.ParamSpec.boolean('screenfull', '', '',
+                    GObject.ParamFlags.READWRITE,
+                    false),
+            };
+
+            static {
+                GObject.registerClass(this);
+            }
+        }
+
+        GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_WARNING,
+            '*property screenfull*');
+
+        const settings = new Gio.Settings({schema: 'org.gnome.GjsTest'});
+        let obj = new SomeObject();
+        settings.bind('fullscreen', obj, 'screenfull', Gio.SettingsBindFlags.DEFAULT);
+        const handler = settings.connect('changed::fullscreen', () => {
+            obj.run_dispose();
+            obj = null;
+            settings.disconnect(handler);
+            GLib.idle_add(GLib.PRIORITY_LOW, () => {
+                GLib.test_assert_expected_messages_internal('Gjs',
+                    'testIntrospection.js', 0,
+                    'Warn about setting property on disposed JS object');
+                done();
+            });
+        });
+        settings.set_boolean('fullscreen', !settings.get_boolean('fullscreen'));
+        settings.reset('fullscreen');
+    });
 });
 
 describe('Gdk.Atom', function () {