diff --git a/0001-Revert-tests-Gtk3-Functions-that-leads-running-dispo.patch b/0001-Revert-tests-Gtk3-Functions-that-leads-running-dispo.patch deleted file mode 100644 index 1b539ea..0000000 --- a/0001-Revert-tests-Gtk3-Functions-that-leads-running-dispo.patch +++ /dev/null @@ -1,40 +0,0 @@ -From c67ca084d7160f836e7dd7f38ee9941718a1fa82 Mon Sep 17 00:00:00 2001 -From: Adam Williamson -Date: Thu, 18 Mar 2021 18:17:54 -0700 -Subject: [PATCH 1/2] Revert "tests/Gtk3: Functions that leads running dispose - vfuncs can be safely called" - -This reverts commit ffac59100c24ac180065e145b7d04c6af5daf927. ---- - installed-tests/js/testGtk3.js | 15 --------------- - 1 file changed, 15 deletions(-) - -diff --git a/installed-tests/js/testGtk3.js b/installed-tests/js/testGtk3.js -index fd0608d1..15187b67 100644 ---- a/installed-tests/js/testGtk3.js -+++ b/installed-tests/js/testGtk3.js -@@ -191,21 +191,6 @@ describe('Gtk overrides', function () { - 'Gtk overrides avoid crashing and print a stack trace'); - }); - -- it('GTK vfuncs can be explicitly called during disposition', function () { -- let called; -- const GoodLabel = GObject.registerClass(class GoodLabel extends Gtk.Label { -- vfunc_destroy() { -- called = true; -- } -- }); -- -- let label = new GoodLabel(); -- label.destroy(); -- expect(called).toBeTruthy(); -- label = null; -- System.gc(); -- }); -- - it('accepts string in place of GdkAtom', function () { - expect(() => Gtk.Clipboard.get(1)).toThrow(); - expect(() => Gtk.Clipboard.get(true)).toThrow(); --- -2.30.2 - diff --git a/0002-Revert-object-Do-not-call-any-function-on-disposed-G.patch b/0002-Revert-object-Do-not-call-any-function-on-disposed-G.patch deleted file mode 100644 index ea05832..0000000 --- a/0002-Revert-object-Do-not-call-any-function-on-disposed-G.patch +++ /dev/null @@ -1,122 +0,0 @@ -From 457de7f08a2ab07ac7e65a6b27e5c9e09403df16 Mon Sep 17 00:00:00 2001 -From: Adam Williamson -Date: Thu, 18 Mar 2021 18:18:00 -0700 -Subject: [PATCH 2/2] Revert "object: Do not call any function on disposed - GObject pointers" - -This reverts commit d37d6604ca0d50b9be408acd637e8073e30500c7. ---- - gi/object.cpp | 15 ++++++--------- - gi/toggle.cpp | 16 +++++++++------- - gi/toggle.h | 2 +- - installed-tests/js/testGtk3.js | 7 +++++-- - 4 files changed, 21 insertions(+), 19 deletions(-) - -diff --git a/gi/object.cpp b/gi/object.cpp -index 68410514..c811a065 100644 ---- a/gi/object.cpp -+++ b/gi/object.cpp -@@ -1297,9 +1297,7 @@ void - ObjectInstance::release_native_object(void) - { - discard_wrapper(); -- if (m_gobj_disposed) -- m_ptr.release(); -- else if (m_uses_toggle_ref) -+ if (m_uses_toggle_ref) - g_object_remove_toggle_ref(m_ptr.release(), wrapped_gobj_toggle_notify, - nullptr); - else -@@ -1481,6 +1479,9 @@ ObjectInstance::disassociate_js_gobject(void) - { - bool had_toggle_down, had_toggle_up; - -+ if (!m_gobj_disposed) -+ g_object_weak_unref(m_ptr.get(), wrapped_gobj_dispose_notify, this); -+ - auto& toggle_queue = ToggleQueue::get_default(); - std::tie(had_toggle_down, had_toggle_up) = toggle_queue.cancel(m_ptr.get()); - if (had_toggle_down != had_toggle_up) { -@@ -1490,12 +1491,8 @@ ObjectInstance::disassociate_js_gobject(void) - m_ptr.get(), type_name()); - } - -- if (!m_gobj_disposed) { -- g_object_weak_unref(m_ptr.get(), wrapped_gobj_dispose_notify, this); -- -- /* Fist, remove the wrapper pointer from the wrapped GObject */ -- unset_object_qdata(); -- } -+ /* Fist, remove the wrapper pointer from the wrapped GObject */ -+ unset_object_qdata(); - - /* Now release all the resources the current wrapper has */ - invalidate_closure_list(&m_closures); -diff --git a/gi/toggle.cpp b/gi/toggle.cpp -index 4f7db32f..5f3d4386 100644 ---- a/gi/toggle.cpp -+++ b/gi/toggle.cpp -@@ -71,17 +71,19 @@ ToggleQueue::is_queued(GObject *gobj) const - return {has_toggle_down, has_toggle_up}; - } - --std::pair ToggleQueue::cancel(GObject* gobj) { -+std::pair -+ToggleQueue::cancel(GObject *gobj) -+{ - debug("cancel", gobj); - std::lock_guard hold(lock); - bool had_toggle_down = find_and_erase_operation_locked(gobj, DOWN); - bool had_toggle_up = find_and_erase_operation_locked(gobj, UP); -- gjs_debug_lifecycle(GJS_DEBUG_GOBJECT, "ToggleQueue: %p was %s", gobj, -- had_toggle_down && had_toggle_up -- ? "queued to toggle BOTH" -- : had_toggle_down ? "queued to toggle DOWN" -- : had_toggle_up ? "queued to toggle UP" -- : "not queued"); -+ gjs_debug_lifecycle(GJS_DEBUG_GOBJECT, "ToggleQueue: %p (%s) was %s", gobj, -+ G_OBJECT_TYPE_NAME(gobj), -+ had_toggle_down && had_toggle_up ? "queued to toggle BOTH" -+ : had_toggle_down ? "queued to toggle DOWN" -+ : had_toggle_up ? "queued to toggle UP" -+ : "not queued"); - return {had_toggle_down, had_toggle_up}; - } - -diff --git a/gi/toggle.h b/gi/toggle.h -index e77c5419..f7f8eac8 100644 ---- a/gi/toggle.h -+++ b/gi/toggle.h -@@ -66,7 +66,7 @@ private: - * are / were queued. is_queued() just checks and does not modify. */ - [[nodiscard]] std::pair is_queued(GObject* gobj) const; - /* Cancels pending toggles and returns whether any were queued. */ -- std::pair cancel(GObject* gobj); -+ std::pair cancel(GObject *gobj); - - /* Pops a toggle from the queue and processes it. Call this if you don't - * want to wait for it to be processed in idle time. Returns false if queue -diff --git a/installed-tests/js/testGtk3.js b/installed-tests/js/testGtk3.js -index 15187b67..f3f6e724 100644 ---- a/installed-tests/js/testGtk3.js -+++ b/installed-tests/js/testGtk3.js -@@ -180,11 +180,14 @@ describe('Gtk overrides', function () { - GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_CRITICAL, - '*destroy*'); - -- const BadLabel = GObject.registerClass(class BadLabel extends Gtk.Label { -+ let BadLabel = GObject.registerClass(class BadLabel extends Gtk.Label { - vfunc_destroy() {} - }); - -- new BadLabel(); -+ let w = new Gtk.Window(); -+ w.add(new BadLabel()); -+ -+ w.destroy(); - System.gc(); - - GLib.test_assert_expected_messages_internal('Gjs', 'testGtk3.js', 0, --- -2.30.2 - diff --git a/588.patch b/588.patch new file mode 100644 index 0000000..34582b6 --- /dev/null +++ b/588.patch @@ -0,0 +1,70 @@ +From c0003eb5ad4c4b0421d723da3d1ff11991f70fb5 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= +Date: Fri, 19 Mar 2021 16:46:42 +0100 +Subject: [PATCH] object: Ignore toggle notifications after disposition + +As per commit d37d6604 we are not removing a toggle reference on +disposed objects, however it may happen that a disposed object (but not +yet finalized) is still using the toggle references while we're releasing +it, and in such case we must always remove the toggle ref, otherwise +GObject (that doesn't remove toggle notifications on disposition) will +notify us after that the wrapper has been finalized, causing a crash +because at that point the GObject is still very well alive (and so +its qdata) and so when we'll try to get the gjs private data from it, +(namely the JS object wrapper instance) we'll end up accessing freed +memory. + +So, on weak notify callback (that we get during disposition, when the +object memory is still valid like its toggle notifications) remove the +toggle reference that we set and consequently toggle down the JSObject +wrapper, unrooting it so that the garbage collector can pick it (this may +also make it a bit more reactive, without waiting for the last reference +being removed if disposition happens as consequence of a run_dispose() +call). + +We keep the m_uses_toggle_ref set though to avoid adding another one, +but at this point we also have to check whether the object is also +disposed before considering the toggle ref active, and per this in such +case we've to only release (steal) the m_ptr when releasing the native +object not to access to potentially finalized data. + +Fixes: #387 +--- + gi/object.cpp | 10 +++++++++- + 1 file changed, 9 insertions(+), 1 deletion(-) + +diff --git a/gi/object.cpp b/gi/object.cpp +index 68410514..598e6bb0 100644 +--- a/gi/object.cpp ++++ b/gi/object.cpp +@@ -1082,10 +1082,18 @@ static void wrapped_gobj_dispose_notify( + where_the_object_was); + } + ++static void wrapped_gobj_toggle_notify(void*, GObject* gobj, ++ gboolean is_last_ref); ++ + void + ObjectInstance::gobj_dispose_notify(void) + { + m_gobj_disposed = true; ++ ++ if (m_uses_toggle_ref) { ++ g_object_remove_toggle_ref(m_ptr, wrapped_gobj_toggle_notify, nullptr); ++ wrapped_gobj_toggle_notify(nullptr, m_ptr, TRUE); ++ } + } + + void ObjectInstance::iterate_wrapped_gobjects( +@@ -1297,7 +1305,7 @@ void + ObjectInstance::release_native_object(void) + { + discard_wrapper(); +- if (m_gobj_disposed) ++ if (m_uses_toggle_ref && m_gobj_disposed) + m_ptr.release(); + else if (m_uses_toggle_ref) + g_object_remove_toggle_ref(m_ptr.release(), wrapped_gobj_toggle_notify, +-- +GitLab + diff --git a/gjs.spec b/gjs.spec index 53cb462..f8fd2d4 100644 --- a/gjs.spec +++ b/gjs.spec @@ -5,7 +5,7 @@ Name: gjs Version: 1.67.3 -Release: 2%{?dist} +Release: 3%{?dist} Summary: Javascript Bindings for GNOME # The following files contain code from Mozilla which @@ -16,11 +16,11 @@ License: MIT and (MPLv1.1 or GPLv2+ or LGPLv2+) URL: https://wiki.gnome.org/Projects/Gjs Source0: https://download.gnome.org/sources/%{name}/1.67/%{name}-%{version}.tar.xz -# Revert MR #585 to work around frequent crash on unlock: +# Fix a frequent crash on unlock: +# https://gitlab.gnome.org/GNOME/gjs/-/merge_requests/588 # https://bugzilla.redhat.com/show_bug.cgi?id=1940171 # https://gitlab.gnome.org/GNOME/gjs/-/issues/387 -Patch0001: 0001-Revert-tests-Gtk3-Functions-that-leads-running-dispo.patch -Patch0002: 0002-Revert-object-Do-not-call-any-function-on-disposed-G.patch +Patch0: 588.patch BuildRequires: cairo-gobject-devel BuildRequires: dbus-daemon @@ -103,6 +103,9 @@ the functionality of the installed gjs package. %{_datadir}/installed-tests/ %changelog +* Fri Mar 19 2021 Adam Williamson - 1.67.3-3 +- Replace MR #585 reversion with MR #588, hopefully correct fix + * Thu Mar 18 2021 Adam Williamson - 1.67.3-2 - Patches to revert MR #585 to work around frequent crash on unlock