rpms/gjs
7
0
Fork 0
Code Issues Pull Requests Releases Activity

Add patch to fix AppIndicator extension crash

Browse Source
This commit is contained in:
David King 2022-02-23 08:52:47 +00:00
parent 82841de5b8
commit 2398b179cd
2 changed files with 264 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

258
gjs-1.71.1-promise-rejection.patch Normal file
View File

@ -0,0 +1,258 @@
From 3f0c615a802e196f5e14a9819a81e7c74fb6b988 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= <mail@3v1n0.net>
Date: Mon, 21 Feb 2022 17:17:10 +0100
Subject: [PATCH 1/2] context: Only warn about unhandled promises when the job
queue is flushed
A promise rejection may be reported as not handled even though it's
going to be handled by a later added promise. In such case we were
correctly be informed about this via
unregister_unhandled_promise_rejection(), but given that since commit
4446ee7f we promptly warn about unhandled promise rejections our
unhandled rejections stack trace list was cleared earlier than we'd got
an unregistration event.
To avoid this, let's just wait for the job queue to be fully processed,
so that we won't accidentally clear the unhandled rejections list, until
those have been fully processed.
By doing this we are both avoiding misbehaving on the said case, and we
don't regress by keeping the exit-warns handled by #417 still valid.
Added tests to cover both a normal rejection case and the one that was
previously leading to a crash (assertion error).
Fixes: #466
---
gi/closure.cpp | 4 +-
gi/function.cpp | 6 +-
gjs/context.cpp | 6 +-
installed-tests/js/meson.build | 1 +
installed-tests/js/testPromise.js | 106 ++++++++++++++++++++++++++++++
5 files changed, 115 insertions(+), 8 deletions(-)
create mode 100644 installed-tests/js/testPromise.js
diff --git a/gi/closure.cpp b/gi/closure.cpp
index 15b6a5fe7..288550a05 100644
--- a/gi/closure.cpp
+++ b/gi/closure.cpp
@@ -183,7 +183,9 @@ bool Closure::invoke(JS::HandleObject this_obj,
JS::RootedFunction func(m_cx, m_func);
bool ok = JS::Call(m_cx, this_obj, func, args, retval);
GjsContextPrivate* gjs = GjsContextPrivate::from_cx(m_cx);
- gjs->warn_about_unhandled_promise_rejections();
+
+ if (gjs->should_exit(nullptr))
+ gjs->warn_about_unhandled_promise_rejections();
if (!ok) {
/* Exception thrown... */
gjs_debug_closure(
diff --git a/gi/function.cpp b/gi/function.cpp
index f39dc1727..716acc8ec 100644
--- a/gi/function.cpp
+++ b/gi/function.cpp
@@ -379,11 +379,11 @@ void GjsCallbackTrampoline::callback_closure(GIArgument** args, void* result) {
// main loop, or maybe not, but there's no way to tell, so we have
// to exit here instead of propagating the exception back to the
// original calling JS code.
- gjs->warn_about_unhandled_promise_rejections();
-
uint8_t code;
- if (gjs->should_exit(&code))
+ if (gjs->should_exit(&code)) {
+ gjs->warn_about_unhandled_promise_rejections();
exit(code);
+ }
// Some other uncatchable exception, e.g. out of memory
JSFunction* fn = callable();
diff --git a/gjs/context.cpp b/gjs/context.cpp
index 7a6eba6e7..0ba6b250a 100644
--- a/gjs/context.cpp
+++ b/gjs/context.cpp
@@ -1080,10 +1080,7 @@ bool GjsContextPrivate::run_jobs_fallible(GCancellable* cancellable) {
JSAutoRealm ar(m_cx, job);
gjs_debug(GJS_DEBUG_CONTEXT, "handling job %s",
gjs_debug_object(job).c_str());
- bool ok =
- JS::Call(m_cx, JS::UndefinedHandleValue, job, args, &rval);
- warn_about_unhandled_promise_rejections();
- if (!ok) {
+ if (!JS::Call(m_cx, JS::UndefinedHandleValue, job, args, &rval)) {
/* Uncatchable exception - return false so that
* System.exit() works in the interactive shell and when
* exiting the interpreter. */
@@ -1104,6 +1101,7 @@ bool GjsContextPrivate::run_jobs_fallible(GCancellable* cancellable) {
m_draining_job_queue = false;
m_job_queue.clear();
+ warn_about_unhandled_promise_rejections();
JS::JobQueueIsEmpty(m_cx);
return retval;
}
diff --git a/installed-tests/js/meson.build b/installed-tests/js/meson.build
index f531fd356..567e5cc7c 100644
--- a/installed-tests/js/meson.build
+++ b/installed-tests/js/meson.build
@@ -137,6 +137,7 @@ jasmine_tests = [
'Package',
'ParamSpec',
'Print',
+ 'Promise',
'Regress',
'Signals',
'System',
diff --git a/installed-tests/js/testPromise.js b/installed-tests/js/testPromise.js
new file mode 100644
index 000000000..df302bac2
--- /dev/null
+++ b/installed-tests/js/testPromise.js
@@ -0,0 +1,106 @@
+// -*- mode: js; indent-tabs-mode: nil -*-
+// SPDX-License-Identifier: MIT OR LGPL-2.0-or-later
+// SPDX-FileContributor: Authored by: Marco Trevisan <marco.trevisan@canonical.com>
+// SPDX-FileCopyrightText: 2022 Canonical, Ltd.
+
+const {GLib} = imports.gi;
+
+class SubPromise extends Promise {
+ constructor(executor) {
+ super((resolve, reject) => {
+ executor(resolve, reject);
+ });
+ }
+}
+
+const PromiseResult = {
+ RESOLVED: 0,
+ FAILED: 1,
+};
+
+describe('Promise', function () {
+ let loop;
+
+ beforeEach(function () {
+ loop = GLib.MainLoop.new(null, false);
+ });
+
+ function executePromise(promise) {
+ const promiseResult = {};
+
+ promise.then(value => {
+ promiseResult.result = PromiseResult.RESOLVED;
+ promiseResult.value = value;
+ }).catch(e => {
+ promiseResult.result = PromiseResult.FAILED;
+ promiseResult.error = e;
+ });
+
+ while (promiseResult.result === undefined)
+ loop.get_context().iteration(true);
+
+ return promiseResult;
+ }
+
+ it('waits for all promises before handling unhandled, when handled', function () {
+ let error;
+ let resolved;
+
+ const promise = async () => {
+ await new SubPromise(resolve => resolve('success'));
+ const rejecting = new SubPromise((_resolve, reject) => reject('got error'));
+ try {
+ await rejecting;
+ } catch (e) {
+ error = e;
+ } finally {
+ resolved = true;
+ }
+
+ expect(resolved).toBeTrue();
+ expect(error).toBe('got error');
+
+ return 'parent-returned';
+ };
+
+ expect(executePromise(promise())).toEqual({
+ result: PromiseResult.RESOLVED,
+ value: 'parent-returned',
+ });
+ });
+
+ it('waits for all promises before handling unhandled, when unhandled', function () {
+ const thenHandler = jasmine.createSpy('thenHandler');
+
+ const promise = async () => {
+ await new Promise(resolve => resolve('success'));
+ await new Promise((_resolve, reject) => reject(new Error('got error')));
+ return 'parent-returned';
+ };
+
+ promise().then(thenHandler).catch();
+ expect(thenHandler).not.toHaveBeenCalled();
+
+ GLib.idle_add(GLib.PRIORITY_DEFAULT_IDLE, () => loop.quit());
+ GLib.test_expect_message('Gjs', GLib.LogLevelFlags.LEVEL_WARNING,
+ 'Unhandled promise rejection.*');
+ loop.run();
+ GLib.test_assert_expected_messages_internal('Gjs', 'testPromise.js', 0,
+ 'warnsIfRejected');
+ });
+
+ it('do not lead to high-priority IDLE starvation', function () {
+ const promise = new Promise(resolve => {
+ const id = GLib.idle_add(GLib.PRIORITY_HIGH, () => {
+ resolve();
+ return GLib.SOURCE_REMOVE;
+ });
+ GLib.Source.set_name_by_id(id, `Test Idle source ${id}`);
+ });
+
+ expect(executePromise(promise)).toEqual({
+ result: PromiseResult.RESOLVED,
+ value: undefined,
+ });
+ });
+});
--
GitLab
From 865d2ae27b853848157215cc60e4888575f13200 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= <mail@3v1n0.net>
Date: Tue, 22 Feb 2022 04:18:40 +0100
Subject: [PATCH 2/2] context: Only show a critical error if unregistering an
unknown promise
In case a promise handled is removed too early or never added we may
have a bug, but not enough critical to crash.
---
gjs/context.cpp | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/gjs/context.cpp b/gjs/context.cpp
index 0ba6b250a..12eb0921c 100644
--- a/gjs/context.cpp
+++ b/gjs/context.cpp
@@ -1150,10 +1150,14 @@ void GjsContextPrivate::register_unhandled_promise_rejection(
}
void GjsContextPrivate::unregister_unhandled_promise_rejection(uint64_t id) {
- // Return value unused in G_DISABLE_ASSERT case
- [[maybe_unused]] size_t erased = m_unhandled_rejection_stacks.erase(id);
- g_assert(((void)"Handler attached to rejected promise that wasn't "
- "previously marked as unhandled", erased == 1));
+ size_t erased = m_unhandled_rejection_stacks.erase(id);
+ if (erased != 1) {
+ g_critical("Promise %" G_GUINT64_FORMAT
+ " Handler attached to rejected promise that wasn't "
+ "previously marked as unhandled or that we wrongly reported "
+ "as unhandled",
+ id);
+ }
}
void GjsContextPrivate::async_closure_enqueue_for_gc(Gjs::Closure* trampoline) {
--
GitLab

7
gjs.spec
View File

@ -4,7 +4,7 @@
Name: gjs
Version: 1.71.1
Release: 3%{?dist}
Release: 4%{?dist}
Summary: Javascript Bindings for GNOME
# The following files contain code from Mozilla which
@ -18,6 +18,8 @@ Source0: https://download.gnome.org/sources/%{name}/1.71/%{name}-%{versio
Patch0: gjs-1.71.1-cairo-textextents-test.patch
# https://gitlab.gnome.org/GNOME/gjs/-/issues/462
Patch1: gjs-1.71.1-bigint-32bit-test.patch
# https://gitlab.gnome.org/GNOME/gjs/-/merge_requests/729
Patch2: gjs-1.71.1-promise-rejection.patch
BuildRequires: gcc-c++
BuildRequires: meson
@ -101,6 +103,9 @@ sed -i -e "/'Gtk4'/d" installed-tests/js/meson.build
%{_datadir}/installed-tests/
%changelog
* Wed Feb 23 2022 David King <amigadave@amigadave.com> - 1.71.1-4
- Add patch to fix AppIndicator extension crash
* Mon Feb 21 2022 David King <amigadave@amigadave.com> - 1.71.1-3
- Fix 32-bit GI marshalling test