Commit dc31b7cf authored by Giovanni Campagna's avatar Giovanni Campagna Committed by Giovanni Campagna

Check and prevent reentrancy to the JSAPI during finalization

Keep track of whether the runtime is currently doing GC sweeping,
and prevent calling JS code at that time. This could happen with
dangling signal connections or widgets/actors that are not properly
destroyed, and causes an assert failure with debug libmozjs (and
a crash otherwise)

https://bugzilla.gnome.org/show_bug.cgi?id=725024
parent 3bcd6bac
......@@ -167,6 +167,7 @@ gjs_callback_closure(ffi_cif *cif,
void *data)
{
JSContext *context;
JSRuntime *runtime;
JSObject *global;
GjsCallbackTrampoline *trampoline;
int i, n_args, n_jsargs, n_outargs;
......@@ -181,6 +182,22 @@ gjs_callback_closure(ffi_cif *cif,
gjs_callback_trampoline_ref(trampoline);
context = trampoline->context;
runtime = JS_GetRuntime(context);
if (G_UNLIKELY (gjs_runtime_is_sweeping(runtime))) {
g_critical("Attempting to call back into JSAPI during the sweeping phase of GC. "
"This is most likely caused by not destroying a Clutter actor or Gtk+ "
"widget with ::destroy signals connected, but can also be caused by "
"using the destroy() or dispose() vfuncs. Because it would crash the "
"application, it has been blocked and the JS callback not invoked.");
/* A gjs_dumpstack() would be nice here, but we can't,
because that works by creating a new Error object and
reading the stack property, which is the worst possible
idea during a GC session.
*/
gjs_callback_trampoline_unref(trampoline);
return;
}
JS_BeginRequest(context);
global = JS_GetGlobalObject(context);
JSAutoCompartment ac(context, global);
......
......@@ -58,7 +58,7 @@ closure_marshal(GClosure *closure,
{
JSContext *context;
JSObject *global;
JSRuntime *runtime;
int argc;
jsval *argv;
jsval rval;
......@@ -75,6 +75,31 @@ closure_marshal(GClosure *closure,
}
context = gjs_closure_get_context(closure);
runtime = JS_GetRuntime(context);
if (G_UNLIKELY (gjs_runtime_is_sweeping(runtime))) {
GSignalInvocationHint *hint = (GSignalInvocationHint*) invocation_hint;
g_critical("Attempting to call back into JSAPI during the sweeping phase of GC. "
"This is most likely caused by not destroying a Clutter actor or Gtk+ "
"widget with ::destroy signals connected, but can also be caused by "
"using the destroy() or dispose() vfuncs. Because it would crash the "
"application, it has been blocked and the JS callback not invoked.");
if (hint) {
gpointer instance;
g_signal_query(hint->signal_id, &signal_query);
instance = g_value_peek_pointer(&param_values[0]);
g_critical("The offending signal was %s on %s %p.", signal_query.signal_name,
g_type_name(G_TYPE_FROM_INSTANCE(instance)), instance);
}
/* A gjs_dumpstack() would be nice here, but we can't,
because that works by creating a new Error object and
reading the stack property, which is the worst possible
idea during a GC session.
*/
return;
}
JS_BeginRequest(context);
global = JS_GetGlobalObject(context);
JSAutoCompartment ac(context, global);
......
......@@ -24,6 +24,19 @@
#include <config.h>
#include "compat.h"
#include "runtime.h"
struct RuntimeData {
JSBool in_gc_sweep;
};
JSBool
gjs_runtime_is_sweeping (JSRuntime *runtime)
{
RuntimeData *data = (RuntimeData*) JS_GetRuntimePrivate(runtime);
return data->in_gc_sweep;
}
/* Implementations of locale-specific operations; these are used
* in the implementation of String.localeCompare(), Date.toLocaleDateString(),
......@@ -137,6 +150,9 @@ static void
destroy_runtime(gpointer data)
{
JSRuntime *runtime = (JSRuntime *) data;
RuntimeData *rtdata = (RuntimeData *) JS_GetRuntimePrivate(runtime);
g_free(rtdata);
JS_DestroyRuntime(runtime);
}
......@@ -150,19 +166,81 @@ static JSLocaleCallbacks gjs_locale_callbacks =
gjs_locale_to_unicode
};
void
gjs_finalize_callback(JSFreeOp *fop,
JSFinalizeStatus status,
JSBool isCompartment)
{
JSRuntime *runtime;
RuntimeData *data;
runtime = fop->runtime();
data = (RuntimeData*) JS_GetRuntimePrivate(runtime);
/* Implementation note for mozjs 24:
sweeping happens in two phases, in the first phase all
GC things from the allocation arenas are queued for
sweeping, then the actual sweeping happens.
The first phase is marked by JSFINALIZE_GROUP_START,
the second one by JSFINALIZE_GROUP_END, and finally
we will see JSFINALIZE_COLLECTION_END at the end of
all GC.
(see jsgc.cpp, BeginSweepPhase/BeginSweepingZoneGroup
and SweepPhase, all called from IncrementalCollectSlice).
Incremental GC muds the waters, because BeginSweepPhase
is always run to entirety, but SweepPhase can be run
incrementally and mixed with JS code runs or even
native code, when MaybeGC/IncrementalGC return.
Luckily for us, objects are treated specially, and
are not really queued for deferred incremental
finalization (unless they are marked for background
sweeping). Instead, they are finalized immediately
during phase 1, so the following guarantees are
true (and we rely on them)
- phase 1 of GC will begin and end in the same JSAPI
call (ie, our callback will be called with GROUP_START
and the triggering JSAPI call will not return until
we see a GROUP_END)
- object finalization will begin and end in the same
JSAPI call
- therefore, if there is a finalizer frame somewhere
in the stack, gjs_runtime_is_sweeping() will return
TRUE.
Comments in mozjs24 imply that this behavior might
change in the future, but it hasn't changed in
mozilla-central as of 2014-02-23. In addition to
that, the mozilla-central version has a huge comment
in a different portion of the file, explaining
why finalization of objects can't be mixed with JS
code, so we can probably rely on this behavior.
*/
if (status == JSFINALIZE_GROUP_START)
data->in_gc_sweep = JS_TRUE;
else if (status == JSFINALIZE_GROUP_END)
data->in_gc_sweep = JS_FALSE;
}
JSRuntime *
gjs_runtime_for_current_thread(void)
{
JSRuntime *runtime = (JSRuntime *) g_private_get(&thread_runtime);
RuntimeData *data;
if (!runtime) {
runtime = JS_NewRuntime(32*1024*1024 /* max bytes */, JS_USE_HELPER_THREADS);
if (runtime == NULL)
g_error("Failed to create javascript runtime");
data = g_new0(RuntimeData, 1);
JS_SetRuntimePrivate(runtime, data);
JS_SetNativeStackQuota(runtime, 1024*1024);
JS_SetGCParameter(runtime, JSGC_MAX_BYTES, 0xffffffff);
JS_SetLocaleCallbacks(runtime, &gjs_locale_callbacks);
JS_SetFinalizeCallback(runtime, gjs_finalize_callback);
g_private_set(&thread_runtime, runtime);
}
......
......@@ -26,4 +26,6 @@
JSRuntime * gjs_runtime_for_current_thread (void);
JSBool gjs_runtime_is_sweeping (JSRuntime *runtime);
#endif /* __GJS_RUNTIME_H__ */
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment