[gjs/wip/ptomato/classes: 1/4] promise: Move to native promises
- From: Philip Chimento <pchimento src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [gjs/wip/ptomato/classes: 1/4] promise: Move to native promises
- Date: Tue, 18 Jul 2017 00:29:05 +0000 (UTC)
commit 55a668a3722be23fbcefd802a78bc5c2e0a65eb5
Author: Philip Chimento <philip chimento gmail com>
Date: Sat Jul 8 19:23:47 2017 -0700
promise: Move to native promises
SpiderMonkey 52 includes native promises, so we can remove the embedded
JS implementation of promises. Queueing up and resolving of promises is
now done inside SpiderMonkey itself, and we must implement some
SpiderMonkey extension points in order to get this to integrate with our
GLib main loops.
The implementation is adapted from what SpiderMonkey does internally in
its js::RunJobs() function; see the code in jscntxt.cpp.
https://bugzilla.gnome.org/show_bug.cgi?id=784713
gjs/context-private.h | 5 +
gjs/context.cpp | 129 ++++++++++-
gjs/engine.cpp | 13 +
gjs/global.cpp | 50 +----
installed-tests/scripts/testCommandLine.sh | 23 ++-
modules/_lie.js | 344 ----------------------------
modules/console.cpp | 19 +-
modules/modules.gresource.xml | 1 -
8 files changed, 183 insertions(+), 401 deletions(-)
---
diff --git a/gjs/context-private.h b/gjs/context-private.h
index d6207da..824c47e 100644
--- a/gjs/context-private.h
+++ b/gjs/context-private.h
@@ -48,6 +48,11 @@ void _gjs_context_set_sweeping(GjsContext *js_context,
bool _gjs_context_is_sweeping(JSContext *cx);
+bool _gjs_context_enqueue_job(GjsContext *gjs_context,
+ JS::HandleObject job);
+
+bool _gjs_context_run_jobs(GjsContext *gjs_context);
+
G_END_DECLS
#endif /* __GJS_CONTEXT_PRIVATE_H__ */
diff --git a/gjs/context.cpp b/gjs/context.cpp
index b453fcc..9aecf13 100644
--- a/gjs/context.cpp
+++ b/gjs/context.cpp
@@ -63,6 +63,9 @@ static void gjs_context_set_property (GObject *object,
guint prop_id,
const GValue *value,
GParamSpec *pspec);
+
+using JobQueue = JS::GCVector<JSObject *, 0, js::SystemAllocPolicy>;
+
struct _GjsContext {
GObject parent;
@@ -83,6 +86,10 @@ struct _GjsContext {
guint auto_gc_id;
std::array<JS::PersistentRootedId*, GJS_STRING_LAST> const_strings;
+
+ JS::PersistentRooted<JobQueue> *job_queue;
+ unsigned idle_drain_handler;
+ bool draining_job_queue;
};
/* Keep this consistent with GjsConstString */
@@ -227,6 +234,8 @@ gjs_context_dispose(GObject *object)
for (auto& root : js_context->const_strings)
delete root;
+ delete js_context->job_queue;
+
/* Tear down JS */
JS_DestroyContext(js_context->context);
js_context->context = NULL;
@@ -281,6 +290,10 @@ gjs_context_constructed(GObject *object)
gjs_intern_string_to_id(cx, const_strings[i]));
}
+ js_context->job_queue = new JS::PersistentRooted<JobQueue>(cx);
+ if (!js_context->job_queue)
+ g_error("Failed to initialize promise job queue");
+
JS_BeginRequest(cx);
JS::RootedObject global(cx, gjs_create_global_object(cx));
@@ -446,6 +459,111 @@ _gjs_context_is_sweeping(JSContext *cx)
return js_context->in_gc_sweep;
}
+static gboolean
+drain_job_queue_idle_handler(void *data)
+{
+ auto gjs_context = static_cast<GjsContext *>(data);
+ _gjs_context_run_jobs(gjs_context);
+ /* Uncatchable exceptions are swallowed here - no way to get a handle on
+ * the main loop to exit it from this idle handler */
+ gjs_context->idle_drain_handler = 0;
+ return G_SOURCE_REMOVE;
+}
+
+/* See engine.cpp and JS::SetEnqueuePromiseJobCallback(). */
+bool
+_gjs_context_enqueue_job(GjsContext *gjs_context,
+ JS::HandleObject job)
+{
+ if (gjs_context->idle_drain_handler)
+ g_assert(gjs_context->job_queue->length() > 0);
+ else
+ g_assert(gjs_context->job_queue->length() == 0);
+
+ if (!gjs_context->job_queue->append(job))
+ return false;
+ if (!gjs_context->idle_drain_handler)
+ gjs_context->idle_drain_handler =
+ g_idle_add(drain_job_queue_idle_handler, gjs_context);
+
+ return true;
+}
+
+/**
+ * _gjs_context_run_jobs:
+ * @gjs_context: The #GjsContext instance
+ *
+ * Drains the queue of promise callbacks that the JS engine has reported
+ * finished, calling each one and logging any exceptions that it throws.
+ *
+ * Adapted from js::RunJobs() in SpiderMonkey's default job queue
+ * implementation.
+ *
+ * Returns: false if one of the jobs threw an uncatchable exception;
+ * otherwise true.
+ */
+bool
+_gjs_context_run_jobs(GjsContext *gjs_context)
+{
+ bool retval = true;
+ g_assert(gjs_context->job_queue);
+
+ if (gjs_context->draining_job_queue || gjs_context->should_exit)
+ return true;
+
+ auto cx = static_cast<JSContext *>(gjs_context_get_native_context(gjs_context));
+ JSAutoRequest ar(cx);
+
+ gjs_context->draining_job_queue = true; /* Ignore reentrant calls */
+
+ JS::RootedObject job(cx);
+ JS::HandleValueArray args(JS::HandleValueArray::empty());
+ JS::RootedValue rval(cx);
+
+ /* Execute jobs in a loop until we've reached the end of the queue.
+ * Since executing a job can trigger enqueueing of additional jobs,
+ * it's crucial to recheck the queue length during each iteration. */
+ for (size_t ix = 0; ix < gjs_context->job_queue->length(); ix++) {
+ /* A previous job might have set this flag. e.g., System.exit(). */
+ if (gjs_context->should_exit)
+ break;
+
+ job = gjs_context->job_queue->get()[ix];
+
+ /* It's possible that job draining was interrupted prematurely,
+ * leaving the queue partly processed. In that case, slots for
+ * already-executed entries will contain nullptrs, which we should
+ * just skip. */
+ if (!job)
+ continue;
+
+ gjs_context->job_queue->get()[ix] = nullptr;
+ {
+ JSAutoCompartment ac(cx, job);
+ if (!JS::Call(cx, JS::UndefinedHandleValue, job, args, &rval)) {
+ /* Uncatchable exception - return false so that
+ * System.exit() works in the interactive shell and when
+ * exiting the interpreter. */
+ if (!JS_IsExceptionPending(cx)) {
+ retval = false;
+ continue;
+ }
+
+ /* There's nowhere for the exception to go at this point */
+ gjs_log_exception(cx);
+ }
+ }
+ }
+
+ gjs_context->draining_job_queue = false;
+ gjs_context->job_queue->clear();
+ if (gjs_context->idle_drain_handler) {
+ g_source_remove(gjs_context->idle_drain_handler);
+ gjs_context->idle_drain_handler = 0;
+ }
+ return retval;
+}
+
/**
* gjs_context_maybe_gc:
* @context: a #GjsContext
@@ -537,8 +655,15 @@ gjs_context_eval(GjsContext *js_context,
g_object_ref(G_OBJECT(js_context));
JS::RootedValue retval(js_context->context);
- if (!gjs_eval_with_scope(js_context->context, nullptr, script,
- script_len, filename, &retval)) {
+ bool ok = gjs_eval_with_scope(js_context->context, nullptr, script,
+ script_len, filename, &retval);
+
+ /* The promise job queue should be drained even on error, to finish
+ * outstanding async tasks before the context is torn down. Drain after
+ * uncaught exceptions have been reported since draining runs callbacks. */
+ ok = _gjs_context_run_jobs(js_context) && ok;
+
+ if (!ok) {
uint8_t code;
if (_gjs_context_should_exit(js_context, &code)) {
/* exit_status_p is public API so can't be changed, but should be
diff --git a/gjs/engine.cpp b/gjs/engine.cpp
index 30aaeef..ea9ab3e 100644
--- a/gjs/engine.cpp
+++ b/gjs/engine.cpp
@@ -209,6 +209,17 @@ on_garbage_collect(JSContext *cx,
gjs_object_clear_toggles();
}
+static bool
+on_enqueue_promise_job(JSContext *cx,
+ JS::HandleObject callback,
+ JS::HandleObject allocation_site,
+ JS::HandleObject global,
+ void *data)
+{
+ auto gjs_context = static_cast<GjsContext *>(data);
+ return _gjs_context_enqueue_job(gjs_context, callback);
+}
+
#ifdef G_OS_WIN32
HMODULE gjs_dll;
static bool gjs_is_inited = false;
@@ -292,6 +303,8 @@ gjs_create_js_context(GjsContext *js_context)
JS_SetGCCallback(cx, on_garbage_collect, js_context);
JS_SetLocaleCallbacks(cx, &gjs_locale_callbacks);
JS::SetWarningReporter(cx, gjs_warning_reporter);
+ JS::SetGetIncumbentGlobalCallback(cx, gjs_get_import_global);
+ JS::SetEnqueuePromiseJobCallback(cx, on_enqueue_promise_job, js_context);
/* setExtraWarnings: Be extra strict about code that might hide a bug */
if (!g_getenv("GJS_DISABLE_EXTRA_WARNINGS")) {
diff --git a/gjs/global.cpp b/gjs/global.cpp
index 295df55..16dd5bd 100644
--- a/gjs/global.cpp
+++ b/gjs/global.cpp
@@ -201,44 +201,6 @@ class GjsGlobal {
JS_FS_END
};
- static bool
- define_promise_object(JSContext *cx,
- JS::HandleObject global)
- {
- /* This is not a regular import, we just load the module's code from the
- * GResource and evaluate it */
-
- GError *error = NULL;
- GBytes *lie_bytes = g_resources_lookup_data("/org/gnome/gjs/modules/_lie.js",
- G_RESOURCE_LOOKUP_FLAGS_NONE,
- &error);
- if (lie_bytes == NULL) {
- g_critical("Failed to load Promise resource: %s", error->message);
- g_clear_error(&error);
- return false;
- }
-
- /* It should be OK to cast these bytes to const char *, since the module is
- * a text file and we setUTF8(true) below */
- size_t lie_length;
- const char *lie_code = static_cast<const char *>(g_bytes_get_data(lie_bytes,
- &lie_length));
- JS::CompileOptions options(cx);
- options.setUTF8(true)
- .setSourceIsLazy(true)
- .setFile("<Promise>");
-
- JS::RootedValue promise(cx);
- if (!JS::Evaluate(cx, options, lie_code, lie_length, &promise)) {
- g_bytes_unref(lie_bytes);
- return false;
- }
- g_bytes_unref(lie_bytes);
-
- return JS_DefineProperty(cx, global, "Promise", promise,
- JSPROP_READONLY | JSPROP_PERMANENT);
- }
-
public:
static JSObject *
@@ -280,15 +242,9 @@ public:
/* Wrapping is a no-op if the importer is already in the same
* compartment. */
- if (!JS_WrapObject(cx, &root_importer) ||
- !gjs_object_define_property(cx, global, GJS_STRING_IMPORTS,
- root_importer, GJS_MODULE_PROP_FLAGS))
- return false;
-
- /* FIXME: We should define the Promise object before any imports, in
- * case the imports want to use it. Currently that's not possible as it
- * needs to import GLib */
- return define_promise_object(cx, global);
+ return JS_WrapObject(cx, &root_importer) &&
+ gjs_object_define_property(cx, global, GJS_STRING_IMPORTS,
+ root_importer, GJS_MODULE_PROP_FLAGS);
}
};
diff --git a/installed-tests/scripts/testCommandLine.sh b/installed-tests/scripts/testCommandLine.sh
index 2e0d55e..096b793 100755
--- a/installed-tests/scripts/testCommandLine.sh
+++ b/installed-tests/scripts/testCommandLine.sh
@@ -27,6 +27,18 @@ if (ARGV.indexOf('--help') == -1)
System.exit(0);
EOF
+# this JS script should print one string (jobs are run before the interpreter
+# finishes) and should not print the other (jobs should not be run after the
+# interpreter is instructed to quit)
+cat <<EOF >promise.js
+const System = imports.system;
+Promise.resolve().then(() => {
+ print('Should be printed');
+ System.exit(42);
+});
+Promise.resolve().then(() => print('Should not be printed'));
+EOF
+
total=0
report () {
@@ -126,6 +138,15 @@ report "--version after -c should be passed to script"
test -z "$("$gjs" -c "$script" --version)"
report "--version after -c should not print anything"
-rm -f exit.js help.js
+# interpreter handles queued promise jobs correctly
+output=$("$gjs" promise.js)
+test $? -eq 42
+report "interpreter should exit with the correct exit code from a queued promise job"
+test -n "$output" -a -z "${output##*Should be printed*}"
+report "interpreter should run queued promise jobs before finishing"
+test -n "${output##*Should not be printed*}"
+report "interpreter should stop running jobs when one calls System.exit()"
+
+rm -f exit.js help.js promise.js
echo "1..$total"
diff --git a/modules/console.cpp b/modules/console.cpp
index 77e237e..794ee1e 100644
--- a/modules/console.cpp
+++ b/modules/console.cpp
@@ -54,6 +54,7 @@
#include "console.h"
#include "gjs/context.h"
+#include "gjs/context-private.h"
#include "gjs/jsapi-private.h"
#include "gjs/jsapi-wrapper.h"
@@ -279,19 +280,25 @@ gjs_console_interact(JSContext *context,
} while (!JS_BufferIsCompilableUnit(context, global,
buffer->str, buffer->len));
- AutoReportException are(context);
- if (!gjs_console_eval_and_print(context, buffer->str, buffer->len,
- startline)) {
+ bool ok;
+ {
+ AutoReportException are(context);
+ ok = gjs_console_eval_and_print(context, buffer->str, buffer->len,
+ startline);
+ }
+ g_string_free(buffer, true);
+
+ auto gjs_context = static_cast<GjsContext *>(JS_GetContextPrivate(context));
+ ok = _gjs_context_run_jobs(gjs_context) && ok;
+
+ if (!ok) {
/* If this was an uncatchable exception, throw another uncatchable
* exception on up to the surrounding JS::Evaluate() in main(). This
* happens when you run gjs-console and type imports.system.exit(0);
* at the prompt. If we don't throw another uncatchable exception
* here, then it's swallowed and main() won't exit. */
- g_string_free(buffer, true);
return false;
}
-
- g_string_free(buffer, true);
} while (!eof);
g_fprintf(stdout, "\n");
diff --git a/modules/modules.gresource.xml b/modules/modules.gresource.xml
index 6aa5eb8..8d2849c 100644
--- a/modules/modules.gresource.xml
+++ b/modules/modules.gresource.xml
@@ -14,7 +14,6 @@
<file>modules/coverage.js</file>
<file>modules/gettext.js</file>
<file>modules/lang.js</file>
- <file>modules/_lie.js</file>
<file>modules/mainloop.js</file>
<file>modules/jsUnit.js</file>
<file>modules/signals.js</file>
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]
