92 lines
3.7 KiB
Diff
92 lines
3.7 KiB
Diff
From cf128c8d52a3b86177d5231f32c2e156837fa8e7 Mon Sep 17 00:00:00 2001
|
|
From: Philip Chimento <philip.chimento@gmail.com>
|
|
Date: Mon, 14 Nov 2022 22:01:59 -0800
|
|
Subject: [PATCH] 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
|
|
---
|
|
gi/gobject.cpp | 12 ++++++++++
|
|
installed-tests/js/testIntrospection.js | 31 +++++++++++++++++++++++++
|
|
2 files changed, 43 insertions(+)
|
|
|
|
diff --git a/gi/gobject.cpp b/gi/gobject.cpp
|
|
index b86872c2..881c06f8 100644
|
|
--- a/gi/gobject.cpp
|
|
+++ b/gi/gobject.cpp
|
|
@@ -171,6 +171,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_context();
|
|
|
|
JS::RootedObject js_obj(cx, priv->wrapper());
|
|
@@ -184,6 +190,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_context();
|
|
|
|
JS::RootedObject js_obj(cx, priv->wrapper());
|
|
diff --git a/installed-tests/js/testIntrospection.js b/installed-tests/js/testIntrospection.js
|
|
index 5e2ee7df..a0ffeefe 100644
|
|
--- a/installed-tests/js/testIntrospection.js
|
|
+++ b/installed-tests/js/testIntrospection.js
|
|
@@ -140,6 +140,37 @@ 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) {
|
|
+ const SomeObject = GObject.registerClass({
|
|
+ Properties: {
|
|
+ 'screenfull': GObject.ParamSpec.boolean('screenfull', '', '',
|
|
+ GObject.ParamFlags.READWRITE,
|
|
+ false),
|
|
+ },
|
|
+ }, class SomeObject extends GObject.Object {});
|
|
+
|
|
+ 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 () {
|
|
--
|
|
2.39.1
|
|
|