[gjs] function: Perform dirty exit at FFI boundary

[Philip Chimento <pchimento src gnome org>]

commit c7bdcaab46f8726078181db338ddc1a07f6a1f82
Author: Philip Chimento <philip endlessm com>
Date:   Tue Mar 7 18:24:33 2017 -0800

    function: Perform dirty exit at FFI boundary
    
    If an "uncatchable" (SpiderMonkey term) exception is thrown (that is,
    return false from a JSAPI call without an exception pending on the
    JSContext), then we have to deal with the condition when returning back
    to C code. We use the uncatchable exception mechanism to implement
    System.exit(), and so gjs_context_eval() checks for this and exits if
    appropriate.
    
    However, when calling System.exit() inside a callback marshalled by FFI,
    the uncatchable exception propagates up to gjs_callback_closure() and
    stopes there. There is nothing that can propagate the exception after
    that point. There may be one or more main loops running, and until they
    exit, we don't even get back to our JSAPI code that will propagate the
    exception up to gjs_context_eval().
    
    Therefore, we do a "dirty exit" (our term) if an FFI callback throws an
    uncatchable exception. This will, unfortunately, trip an assertion if
    SpiderMonkey is compiled in debug mode, because we haven't cleaned up the
    JSContext by the time the JSRuntime is destroyed when the thread exits.
    We can't clean up the JSContext, either, because the caller of
    gjs_context_eval() is still holding a request on the JSContext so if we
    did that we would trip a different assertion.
    
    This SpiderMonkey limitation is reflected in the added test, whereby the
    test will still pass if the script exits with any nonzero code, which
    covers the debug-mode case where we fail the assertion.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=779692

 gi/function.cpp                            |   15 +++++++++++++++
 gjs/context-private.h                      |    3 +++
 gjs/context.cpp                            |    8 ++++----
 installed-tests/scripts/testCommandLine.sh |   18 +++++++++++++++++-
 4 files changed, 39 insertions(+), 5 deletions(-)
---
diff --git a/gi/function.cpp b/gi/function.cpp
index 93b8cdb..7a0fc58 100644
--- a/gi/function.cpp
+++ b/gi/function.cpp
@@ -33,6 +33,7 @@
 #include "closure.h"
 #include "gtype.h"
 #include "param.h"
+#include "gjs/context-private.h"
 #include "gjs/jsapi-private.h"
 #include "gjs/jsapi-wrapper.h"
 #include "gjs/mem.h"
@@ -395,6 +396,20 @@ gjs_callback_closure(ffi_cif *cif,
 
 out:
     if (!success) {
+        if (!JS_IsExceptionPending(context)) {
+            /* "Uncatchable" exception thrown, we have to exit. We may be in a
+             * 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. */
+            auto gcx = static_cast<GjsContext *>(JS_GetContextPrivate(context));
+            uint8_t code;
+            if (_gjs_context_should_exit(gcx, &code))
+                exit(code);
+
+            /* Some other uncatchable exception, e.g. out of memory */
+            exit(1);
+        }
+
         gjs_log_exception (context);
 
         /* Fill in the result with some hopefully neutral value */
diff --git a/gjs/context-private.h b/gjs/context-private.h
index 5bbcfa7..6241f5a 100644
--- a/gjs/context-private.h
+++ b/gjs/context-private.h
@@ -39,6 +39,9 @@ void _gjs_context_exit(GjsContext *js_context,
 
 bool _gjs_context_get_is_owner_thread(GjsContext *js_context);
 
+bool _gjs_context_should_exit(GjsContext *js_context,
+                              uint8_t    *exit_code_p);
+
 G_END_DECLS
 
 #endif  /* __GJS_CONTEXT_PRIVATE_H__ */
diff --git a/gjs/context.cpp b/gjs/context.cpp
index f0ce8e7..30c9ca6 100644
--- a/gjs/context.cpp
+++ b/gjs/context.cpp
@@ -649,9 +649,9 @@ _gjs_context_exit(GjsContext *js_context,
     js_context->exit_code = exit_code;
 }
 
-static bool
-context_should_exit(GjsContext *js_context,
-                    uint8_t    *exit_code_p)
+bool
+_gjs_context_should_exit(GjsContext *js_context,
+                         uint8_t    *exit_code_p)
 {
     if (exit_code_p != NULL)
         *exit_code_p = js_context->exit_code;
@@ -765,7 +765,7 @@ gjs_context_eval(GjsContext   *js_context,
     if (!gjs_eval_with_scope(js_context->context, JS::NullPtr(), script,
                              script_len, filename, &retval)) {
         uint8_t code;
-        if (context_should_exit(js_context, &code)) {
+        if (_gjs_context_should_exit(js_context, &code)) {
             /* exit_status_p is public API so can't be changed, but should be
              * uint8_t, not int */
             *exit_status_p = code;
diff --git a/installed-tests/scripts/testCommandLine.sh b/installed-tests/scripts/testCommandLine.sh
index 5de7604..e76ea29 100755
--- a/installed-tests/scripts/testCommandLine.sh
+++ b/installed-tests/scripts/testCommandLine.sh
@@ -6,6 +6,16 @@ else
     gjs=gjs-console
 fi
 
+# This JS script should exit immediately with code 42. If that is not working,
+# then it will exit after 3 seconds as a fallback, with code 0.
+cat <<EOF >exit.js
+const GLib = imports.gi.GLib;
+let loop = GLib.MainLoop.new(null, false);
+GLib.idle_add(GLib.PRIORITY_LOW, () => imports.system.exit(42));
+GLib.timeout_add_seconds(GLib.PRIORITY_HIGH, 3, () => loop.quit());
+loop.run();
+EOF
+
 # this JS script fails if either 1) --help is not passed to it, or 2) the string
 # "sentinel" is not in its search path
 cat <<EOF >help.js
@@ -46,6 +56,12 @@ report "System.exit(0) should exit successfully"
 test $? -eq 42
 report "System.exit(42) should exit with the correct exit code"
 
+# FIXME: should check -eq 42 specifically, but in debug mode we will be
+# hitting an assertion
+"$gjs" exit.js
+test $? -ne 0
+report "System.exit() should still exit across an FFI boundary"
+
 # gjs --help prints GJS help
 "$gjs" --help >/dev/null
 report "--help should succeed"
@@ -110,6 +126,6 @@ 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 help.js
+rm -f exit.js help.js
 
 echo "1..$total"