1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
|
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
|
