diff --git a/MANIFEST b/MANIFEST index 71f6e9e..47f2132 100644 --- a/MANIFEST +++ b/MANIFEST @@ -57,3 +57,4 @@ t/21_this_bug.t t/22_overflow.t t/23_dualvar.t t/24_version.t +t/25_die_in_callback.t diff --git a/V8Context.cc b/V8Context.cc index 232672d..c9de581 100644 --- a/V8Context.cc +++ b/V8Context.cc @@ -29,6 +29,27 @@ int V8Context::instance_count = 0; std::unique_ptr V8Context::platform = 0; +/* + * Raise a deferred error (see pl_util.h). This must only be called once + * every V8 scope object (ENTER_SCOPE, HandleScope, ...) has gone out of + * scope, which is why the public methods below run their body in a nested + * block and call this afterwards. If 'leak' is given it is released first, + * because the XS glue will never see it once we croak. + */ +static void raise_pending_error(SV* leak = 0) +{ + if (!pl_has_pending_error()) { + return; + } + dTHX; + if (leak && !SvIMMORTAL(leak)) { + SvREFCNT_dec(leak); + } + SV* msg = sv_2mortal(newSVpv(pl_get_pending_error(), 0)); + pl_clear_pending_error(); + croak_sv(msg); +} + V8Context::V8Context(HV* opt) : isolate(0), persistent_context(0), @@ -92,6 +113,7 @@ V8Context::V8Context(HV* opt) create_params.array_buffer_allocator = ArrayBuffer::Allocator::NewDefaultAllocator(); set_up(); + raise_pending_error(); } V8Context::~V8Context() @@ -112,116 +134,155 @@ V8Context::~V8Context() SV* V8Context::get(const char* name) { - ENTER_SCOPE; - set_up(); - - Perf perf; - pl_stats_start(aTHX_ this, &perf); - SV* ret = pl_get_global_or_property(aTHX_ this, name); - pl_stats_stop(aTHX_ this, &perf, "get"); + SV* ret = 0; + { + ENTER_SCOPE; + set_up(); + + Perf perf; + pl_stats_start(aTHX_ this, &perf); + ret = pl_get_global_or_property(aTHX_ this, name); + pl_stats_stop(aTHX_ this, &perf, "get"); + } + raise_pending_error(ret); return ret; } SV* V8Context::exists(const char* name) { - ENTER_SCOPE; - set_up(); - - Perf perf; - pl_stats_start(aTHX_ this, &perf); - SV* ret = pl_exists_global_or_property(aTHX_ this, name); - pl_stats_stop(aTHX_ this, &perf, "exists"); + SV* ret = 0; + { + ENTER_SCOPE; + set_up(); + + Perf perf; + pl_stats_start(aTHX_ this, &perf); + ret = pl_exists_global_or_property(aTHX_ this, name); + pl_stats_stop(aTHX_ this, &perf, "exists"); + } + raise_pending_error(ret); return ret; } SV* V8Context::typeof(const char* name) { - ENTER_SCOPE; - set_up(); - - Perf perf; - pl_stats_start(aTHX_ this, &perf); - SV* ret = pl_typeof_global_or_property(aTHX_ this, name); - pl_stats_stop(aTHX_ this, &perf, "typeof"); + SV* ret = 0; + { + ENTER_SCOPE; + set_up(); + + Perf perf; + pl_stats_start(aTHX_ this, &perf); + ret = pl_typeof_global_or_property(aTHX_ this, name); + pl_stats_stop(aTHX_ this, &perf, "typeof"); + } + raise_pending_error(ret); return ret; } SV* V8Context::instanceof(const char* oname, const char* cname) { - ENTER_SCOPE; - set_up(); - - Perf perf; - pl_stats_start(aTHX_ this, &perf); - SV* ret = pl_instanceof_global_or_property(aTHX_ this, oname, cname); - pl_stats_stop(aTHX_ this, &perf, "instanceof"); + SV* ret = 0; + { + ENTER_SCOPE; + set_up(); + + Perf perf; + pl_stats_start(aTHX_ this, &perf); + ret = pl_instanceof_global_or_property(aTHX_ this, oname, cname); + pl_stats_stop(aTHX_ this, &perf, "instanceof"); + } + raise_pending_error(ret); return ret; } void V8Context::set(const char* name, SV* value) { - ENTER_SCOPE; - set_up(); - - Perf perf; - pl_stats_start(aTHX_ this, &perf); - pl_set_global_or_property(aTHX_ this, name, value); - pl_stats_stop(aTHX_ this, &perf, "set"); + { + ENTER_SCOPE; + set_up(); + + Perf perf; + pl_stats_start(aTHX_ this, &perf); + pl_set_global_or_property(aTHX_ this, name, value); + pl_stats_stop(aTHX_ this, &perf, "set"); + } + raise_pending_error(); } void V8Context::remove(const char* name) { - ENTER_SCOPE; - set_up(); - - Perf perf; - pl_stats_start(aTHX_ this, &perf); - pl_del_global_or_property(aTHX_ this, name); - pl_stats_stop(aTHX_ this, &perf, "remove"); + { + ENTER_SCOPE; + set_up(); + + Perf perf; + pl_stats_start(aTHX_ this, &perf); + pl_del_global_or_property(aTHX_ this, name); + pl_stats_stop(aTHX_ this, &perf, "remove"); + } + raise_pending_error(); } SV* V8Context::eval(const char* code, const char* file) { - ENTER_SCOPE; - set_up(); + SV* ret = 0; + { + ENTER_SCOPE; + set_up(); - /* performance is tracked inside this call */ - return pl_eval(aTHX_ this, code, file); + /* performance is tracked inside this call */ + ret = pl_eval(aTHX_ this, code, file); + } + raise_pending_error(ret); + return ret; } SV* V8Context::dispatch_function_in_event_loop(const char* func) { - ENTER_SCOPE; - set_up(); - - Perf perf; - pl_stats_start(aTHX_ this, &perf); - SV* ret = pl_run_function_in_event_loop(aTHX_ this, func); - pl_stats_stop(aTHX_ this, &perf, "dispatch"); + SV* ret = 0; + { + ENTER_SCOPE; + set_up(); + + Perf perf; + pl_stats_start(aTHX_ this, &perf); + ret = pl_run_function_in_event_loop(aTHX_ this, func); + pl_stats_stop(aTHX_ this, &perf, "dispatch"); + } + raise_pending_error(ret); return ret; } SV* V8Context::global_objects() { - ENTER_SCOPE; - set_up(); - - Perf perf; - pl_stats_start(aTHX_ this, &perf); - SV* ret = pl_global_objects(aTHX_ this); - pl_stats_stop(aTHX_ this, &perf, "global_objects"); + SV* ret = 0; + { + ENTER_SCOPE; + set_up(); + + Perf perf; + pl_stats_start(aTHX_ this, &perf); + ret = pl_global_objects(aTHX_ this); + pl_stats_stop(aTHX_ this, &perf, "global_objects"); + } + raise_pending_error(ret); return ret; } int V8Context::run_gc() { - ENTER_SCOPE; - set_up(); - - Perf perf; - pl_stats_start(aTHX_ this, &perf); - int ret = pl_run_gc(this); - pl_stats_stop(aTHX_ this, &perf, "run_gc"); + int ret = 0; + { + ENTER_SCOPE; + set_up(); + + Perf perf; + pl_stats_start(aTHX_ this, &perf); + ret = pl_run_gc(this); + pl_stats_stop(aTHX_ this, &perf, "run_gc"); + } + raise_pending_error(); return ret; } @@ -332,6 +393,7 @@ void V8Context::reset() { tear_down(); set_up(); + raise_pending_error(); } const char* get_data_path() diff --git a/pl_console.cc b/pl_console.cc index f0aeb8b..cc30047 100644 --- a/pl_console.cc +++ b/pl_console.cc @@ -41,7 +41,8 @@ static void save_msg(pTHX_ V8Context* ctx, const char* target, SV* message) SvREFCNT_inc(pvalue); } else { - croak("Could not store message in target %*.*s\n", (int) tlen, (int) tlen, target); + /* console callbacks run under V8 scopes: never croak() here */ + pl_set_pending_error("Could not store message in target %*.*s\n", (int) tlen, (int) tlen, target); } } diff --git a/pl_stats.cc b/pl_stats.cc index 925a039..f055be7 100644 --- a/pl_stats.cc +++ b/pl_stats.cc @@ -13,7 +13,7 @@ static void save_stat(pTHX_ V8Context* ctx, const char* category, const char* na SV* ref = SvRV(*found); /* value not a valid hashref? bail out */ if (SvTYPE(ref) != SVt_PVHV) { - croak("Found category %s in stats but it is not a hashref\n", category); + pl_set_pending_error("Found category %s in stats but it is not a hashref\n", category); return; } data = (HV*) ref; @@ -25,7 +25,8 @@ static void save_stat(pTHX_ V8Context* ctx, const char* category, const char* na SvREFCNT_inc(ref); } else { - croak("Could not create category %s in stats\n", category); + pl_set_pending_error("Could not create category %s in stats\n", category); + return; } } @@ -34,7 +35,7 @@ static void save_stat(pTHX_ V8Context* ctx, const char* category, const char* na SvREFCNT_inc(pvalue); } else { - croak("Could not create entry %s for category %s in stats\n", name, category); + pl_set_pending_error("Could not create entry %s for category %s in stats\n", name, category); } } diff --git a/pl_util.cc b/pl_util.cc index 16b791a..4800ac0 100644 --- a/pl_util.cc +++ b/pl_util.cc @@ -1,7 +1,41 @@ +#include #include #include #include "pl_util.h" +#define PENDING_ERROR_SIZE 4096 + +static char pending_error[PENDING_ERROR_SIZE]; +static int pending_error_set = 0; + +void pl_set_pending_error(const char* fmt, ...) +{ + if (pending_error_set) { + return; /* keep the first error, it is the most informative one */ + } + va_list ap; + va_start(ap, fmt); + vsnprintf(pending_error, PENDING_ERROR_SIZE, fmt, ap); + va_end(ap); + pending_error_set = 1; +} + +int pl_has_pending_error(void) +{ + return pending_error_set; +} + +const char* pl_get_pending_error(void) +{ + return pending_error; +} + +void pl_clear_pending_error(void) +{ + pending_error_set = 0; + pending_error[0] = '\0'; +} + #define FILE_MEMORY_STATUS "/proc/self/statm" double now_us(void) diff --git a/pl_util.h b/pl_util.h index 366477c..b6226f3 100644 --- a/pl_util.h +++ b/pl_util.h @@ -9,4 +9,25 @@ double now_us(void); /* Get how many memory pages are currently in use */ long total_memory_pages(void); +/* + * Deferred errors. + * + * croak() must NEVER be called while V8 objects with destructors + * (HandleScope, Isolate::Scope, Context::Scope, TryCatch, ...) are on the + * C++ stack: croak() longjmp()s past them, their destructors never run, and + * the isolate is left corrupt (still "entered", stale handle scope data). + * The process then dies later: an abort in Isolate::Dispose() ("Disposing + * the isolate that is entered by a thread") or a SIGSEGV in the next GC + * (Relocatable::Iterate). + * + * Code running under V8 scopes therefore records the error here and returns. + * V8Context methods raise it with croak() once every scope has been closed, + * and perl_caller() turns it into a JS exception. Only the first error is + * kept until it is cleared. + */ +void pl_set_pending_error(const char* fmt, ...); +int pl_has_pending_error(void); +const char* pl_get_pending_error(void); +void pl_clear_pending_error(void); + #endif diff --git a/pl_v8.cc b/pl_v8.cc index 4e7540a..8ede5e6 100644 --- a/pl_v8.cc +++ b/pl_v8.cc @@ -1,4 +1,6 @@ #include +#include +#include "pl_util.h" #include "pl_stats.h" #include "pl_console.h" #include "pl_v8.h" @@ -46,69 +48,117 @@ struct FuncData { static const char* get_typeof(const Local& object); -static bool is_perl_func(const Local &object, const Local &context, Isolate *isolate); - static void perl_caller(const FunctionCallbackInfo& args) { Isolate* isolate = args.GetIsolate(); -#if 1 HandleScope handle_scope(isolate); -#endif -#if 1 Local v8_val = Local::Cast(args.Data()); -#else - /* - * If args.This() returned the same bject as GetFunction() on the function - * template we used to create the function, this would work; alas, it - * doesn't work, so we have to pass the data we want so that args.Data() - * can return it. - */ - Local v8_key = String::NewFromUtf8(isolate, "__perl_callback", NewStringType::kNormal).ToLocalChecked(); - Local v8_func = Local::Cast(args.This()); - Local v8_val = Local::Cast(v8_func->Get(v8_key)); -#endif FuncData* data = (FuncData*) v8_val->Value(); - SV* ret = 0; - SV *err_tmp; + SV* error = 0; /* prepare Perl environment for calling the CV */ dTHX; - dSP; ENTER; SAVETMPS; - PUSHMARK(SP); - /* pass in the stack each of the params we received */ + /* convert each of the params we received; they are all mortal */ int nargs = args.Length(); + std::vector argv; + argv.reserve(nargs); for (int j = 0; j < nargs; j++) { Local arg = Local::Cast(args[j]); Local object = Local::Cast(arg); - SV* val = pl_v8_to_perl(aTHX_ data->ctx, object); - mXPUSHs(val); + SV* val = sv_2mortal(pl_v8_to_perl(aTHX_ data->ctx, object)); + argv.push_back(val); } - /* call actual Perl CV, passing all params */ - PUTBACK; - call_sv(data->func, G_SCALAR | G_EVAL); - SPAGAIN; - - err_tmp = ERRSV; - if (SvTRUE(err_tmp)) { - croak("Perl sub died with error: %s", SvPV_nolen(err_tmp)); + if (pl_has_pending_error()) { + /* could not convert the arguments: do not call the Perl sub at all */ + error = newSVpv(pl_get_pending_error(), 0); + pl_clear_pending_error(); + } + else { + dSP; + PUSHMARK(SP); + EXTEND(SP, nargs); + for (int j = 0; j < nargs; j++) { + PUSHs(argv[j]); + } + PUTBACK; + + /* call actual Perl CV, passing all params */ + call_sv(data->func, G_SCALAR | G_EVAL); + SPAGAIN; + + SV* err_tmp = ERRSV; + if (SvTRUE(err_tmp)) { + /* G_SCALAR|G_EVAL leaves a single undef on the stack when the sub dies */ + (void) POPs; + PUTBACK; + /* copy the error: FREETMPS/LEAVE below may free the original */ + error = newSVpvs("Perl sub died with error: "); + sv_catsv(error, err_tmp); + } + else { + /* get returned value from Perl and return it */ + SV* ret = POPs; + /* + * Restore the Perl stack pointer before converting: the conversion + * may run Perl code (overloaded bool/"" or tied values), which can + * reallocate the stack and make our local SP stale. + */ + PUTBACK; + Local object = pl_perl_to_v8(aTHX_ ret, data->ctx); + if (pl_has_pending_error()) { + error = newSVpv(pl_get_pending_error(), 0); + pl_clear_pending_error(); + } + else { + args.GetReturnValue().Set(object); + } + } } - - /* get returned value from Perl and return it */ - ret = POPs; - Local object = pl_perl_to_v8(aTHX_ ret, data->ctx); - - args.GetReturnValue().Set(object); /* cleanup */ - PUTBACK; FREETMPS; LEAVE; + + if (error) { + /* + * Never call croak() from inside a V8 callback. It longjmp()s through + * the C++ frames of V8 that are on the stack at this point (HandleScope, + * Isolate::Scope, TryCatch, Context::Scope, ...), so their destructors + * never run and the isolate is left "entered". Nothing crashes right + * away, but the next Isolate::Dispose() (V8Context::tear_down) aborts + * the whole process with: + * + * Fatal error in v8::Isolate::Dispose() + * Disposing the isolate that is entered by a thread. + * + * and if the isolate is used again before that, the next GC crashes + * with a SIGSEGV in v8::internal::Relocatable::Iterate. + * + * Instead: + * - throw the error as a JS exception, so that the running script + * unwinds the way any JS error does (JS may even catch it), and + * - record it as the pending error, so that the V8Context method + * that is running the script (eval, dispatch_function_in_event_loop, + * ...) croak()s with it once all V8 scopes are closed. + * The Perl caller therefore still gets "Perl sub died with error: ...", + * exactly as before, just from a place where it is safe to die. + */ + STRLEN elen = 0; + const char* estr = SvPV(error, elen); + Local msg; + if (!String::NewFromUtf8(isolate, estr, NewStringType::kNormal, (int) elen).ToLocal(&msg)) { + msg = String::NewFromUtf8(isolate, "Perl sub died", NewStringType::kNormal).ToLocalChecked(); + } + pl_set_pending_error("%s", estr); + SvREFCNT_dec(error); + isolate->ThrowException(Exception::Error(msg)); + } } static SV* pl_v8_to_perl_impl(pTHX_ V8Context* ctx, const Local& object, MapJ2P& seen) @@ -135,14 +185,22 @@ static SV* pl_v8_to_perl_impl(pTHX_ V8Context* ctx, const Local& object, ret = newSVpvn(*val, val.length()); SvUTF8_on(ret); /* yes, always */ } - else if (object->IsFunction()){ + else if (object->IsFunction()) { Local v8_key = String::NewFromUtf8(ctx->isolate, "__perl_callback", NewStringType::kNormal).ToLocalChecked(); Local key_val; - if (is_perl_func(object, context, ctx->isolate) && object->Get(context, v8_key).ToLocal(&key_val)) { + /* + * Only functions that were created from a Perl CV carry our External + * in "__perl_callback". A plain JS function returns undefined here, + * and casting that to External and dereferencing Value() reads garbage + * memory (a genuine SIGSEGV source), so check IsExternal() first. + * Plain JS functions have no Perl equivalent: they become undef. + */ + if (object->Get(context, v8_key).ToLocal(&key_val) && key_val->IsExternal()) { Local v8_val = Local::Cast(key_val); FuncData* data = (FuncData*) v8_val->Value(); if (data && data->func) { - ret = data->func; + /* the caller owns (mortalizes) the returned SV; FuncData keeps its own reference */ + ret = SvREFCNT_inc_simple_NN(data->func); } } } @@ -163,14 +221,16 @@ static SV* pl_v8_to_perl_impl(pTHX_ V8Context* ctx, const Local& object, for (int j = 0; j < array_top; ++j) { Local value; if (!array->Get(context, j).ToLocal(&value)) { - croak("Could not get array element\n"); + pl_set_pending_error("Could not get array element\n"); + break; } Local elem = Local::Cast(value); /* TODO: check we got a valid element */ SV* nested = sv_2mortal(pl_v8_to_perl_impl(aTHX_ ctx, elem, seen)); if (!nested) { - croak("Could not create Perl SV for array\n"); + pl_set_pending_error("Could not create Perl SV for array\n"); + break; } if (av_store(values_array, j, nested)) { SvREFCNT_inc(nested); @@ -195,13 +255,20 @@ static SV* pl_v8_to_perl_impl(pTHX_ V8Context* ctx, const Local& object, for (int j = 0; j < hash_top; ++j) { Local v8_key; if (!property_names->Get(context, j).ToLocal(&v8_key)) { - croak("Could not get object key\n"); + pl_set_pending_error("Could not get object key\n"); + break; } - String::Utf8Value key(ctx->isolate, v8_key->ToString(context).ToLocalChecked()); + Local key_str; + if (!v8_key->ToString(context).ToLocal(&key_str)) { + pl_set_pending_error("Could not stringify object key\n"); + break; + } + String::Utf8Value key(ctx->isolate, key_str); Local value; if (!object->Get(context, v8_key).ToLocal(&value)) { - croak("Could not get object value key\n"); + pl_set_pending_error("Could not get object value key\n"); + break; } Local obj = Local::Cast(value); @@ -209,7 +276,8 @@ static SV* pl_v8_to_perl_impl(pTHX_ V8Context* ctx, const Local& object, SV* nested = sv_2mortal(pl_v8_to_perl_impl(aTHX_ ctx, obj, seen)); if (!nested) { - croak("Could not create Perl SV for hash\n"); + pl_set_pending_error("Could not create Perl SV for hash\n"); + break; } SV* pkey = newSVpvn(*key, key.length()); SvUTF8_on(pkey); /* yes, always */ @@ -222,25 +290,11 @@ static SV* pl_v8_to_perl_impl(pTHX_ V8Context* ctx, const Local& object, } } else { - croak("Don't know how to deal with this thing\n"); + pl_set_pending_error("Don't know how to deal with this thing\n"); } return ret; } -static bool is_perl_func(const Local& object, const Local& context, Isolate *isolate) -{ - String::Utf8Value detailedName(isolate, object->ToDetailString(context).ToLocalChecked()); - if (!object->IsFunction()) - { - return false; - } - Local func = Local::Cast(object); - - Local v8_key = String::NewFromUtf8(isolate, "__perl_callback", NewStringType::kNormal).ToLocalChecked(); - MaybeLocal v = func->Get(context, v8_key).ToLocalChecked(); - return !(v.IsEmpty() || v.ToLocalChecked()->IsNullOrUndefined()); -} - static const Local pl_perl_to_v8_impl(pTHX_ SV* value, V8Context* ctx, MapP2J& seen, int ref) { Local context = Local::New(ctx->isolate, *ctx->persistent_context); @@ -295,7 +349,8 @@ static const Local pl_perl_to_v8_impl(pTHX_ SV* value, V8Context* ctx, M /* TODO: check for validity */ /* croak("Could not create JS element for array\n"); */ if (!array->Set(context, j, nested).IsJust()) { - croak("Could not set JS element for array\n"); + pl_set_pending_error("Could not set JS element for array\n"); + break; } } } @@ -341,7 +396,8 @@ static const Local pl_perl_to_v8_impl(pTHX_ SV* value, V8Context* ctx, M Local v8_key = String::NewFromUtf8(ctx->isolate, kstr, NewStringType::kNormal).ToLocalChecked(); if (!object->Set(context, v8_key, nested).IsJust()) { - croak("Could not create JS element for hash\n"); + pl_set_pending_error("Could not create JS element for hash\n"); + break; } } } @@ -352,14 +408,14 @@ static const Local pl_perl_to_v8_impl(pTHX_ SV* value, V8Context* ctx, M Local v8_key = String::NewFromUtf8(ctx->isolate, "__perl_callback", NewStringType::kNormal).ToLocalChecked(); Local v8_func = ft->GetFunction(context).ToLocalChecked(); if (!v8_func->Set(context, v8_key, val).IsJust()) { - croak("Could not set JS function pointer"); + pl_set_pending_error("Could not set JS function pointer\n"); } ret = Local::Cast(v8_func); } else { - croak("Don't know how to deal with an undetermined Perl reference\n"); + pl_set_pending_error("Don't know how to deal with an undetermined Perl reference\n"); } } else { - croak("Don't know how to deal with an undetermined Perl object\n"); + pl_set_pending_error("Don't know how to deal with an undetermined Perl object\n"); } return ret; } @@ -409,7 +465,7 @@ int pl_set_global_or_property(pTHX_ V8Context* ctx, const char* name, SV* value) if (found) { Local object = pl_perl_to_v8(aTHX_ value, ctx); if (!parent->Set(context, slot, object).IsJust()) { - croak("could not set global or property"); + pl_set_pending_error("could not set global or property\n"); } ret = 1; } @@ -509,9 +565,15 @@ SV* pl_global_objects(pTHX_ V8Context* ctx) for (uint32_t j = 0; j < property_names->Length(); ++j) { Local v8_key; if (!property_names->Get(context, j).ToLocal(&v8_key)) { - croak("could not get name of global object"); + pl_set_pending_error("could not get name of global object\n"); + break; + } + Local key_str; + if (!v8_key->ToString(context).ToLocal(&key_str)) { + pl_set_pending_error("could not stringify name of global object\n"); + break; } - String::Utf8Value key(ctx->isolate, v8_key->ToString(context).ToLocalChecked()); + String::Utf8Value key(ctx->isolate, key_str); SV* name = sv_2mortal(newSVpvn(*key, key.length())); if (av_store(values, count, name)) { SvREFCNT_inc(name); @@ -554,7 +616,8 @@ bool find_parent(V8Context* ctx, const char* name, Local& context, Loca if (parent->Has(context, slot).ToChecked()) { /* parent has a slot with that name */ if (!parent->Get(context, slot).ToLocal(&child)) { - croak("could not get parent slot"); + pl_set_pending_error("could not get parent slot\n"); + break; } } else if (!create) { @@ -565,7 +628,8 @@ bool find_parent(V8Context* ctx, const char* name, Local& context, Loca /* create the missing slot and go on */ child = Object::New(ctx->isolate); if (!parent->Set(context, slot, child).IsJust()) { - croak("could not set parent slot"); + pl_set_pending_error("could not set parent slot\n"); + break; } } parent = Local::Cast(child); @@ -593,7 +657,7 @@ bool find_object(V8Context* ctx, const char* name, Local& context, Loca } Local child; if (!parent->Get(context, slot).ToLocal(&child)) { - croak("could not get object slot"); + pl_set_pending_error("could not get object slot\n"); return false; } object = Local::Cast(child); diff --git a/t/25_die_in_callback.t b/t/25_die_in_callback.t new file mode 100644 index 0000000..a19582d --- /dev/null +++ b/t/25_die_in_callback.t @@ -0,0 +1,97 @@ +use strict; +use warnings; + +use Test::More; + +my $CLASS = 'JavaScript::V8::XS'; + +# A Perl exception raised inside a function that JS calls back into must never +# croak() through V8's C++ scopes. Before the fix that left the isolate +# "entered": the next VM destruction aborted the whole process inside +# v8::Isolate::Dispose(), and using the VM again first crashed in the next GC. +# +# The contract for the Perl caller is unchanged: $vm->eval() still dies with +# "Perl sub died with error: ...", but only after V8 has unwound. Inside JS +# the Perl error is a regular exception. + +sub test_perl_die_still_reaches_the_perl_caller { + my $vm = $CLASS->new({ save_messages => 1 }); + $vm->set('boom', sub { die "kaboom\n" }); + $vm->set('fine', sub { return 'ok' }); + + my $ok = eval { $vm->eval('var caught; try { boom() } catch (e) { caught = e.message }; "done"', 'die1.js'); 1 }; + ok(!$ok, 'eval() dies when a callback dies'); + like($@, qr/^Perl sub died with error: kaboom/, 'with the same message as before'); + like($vm->get('caught'), qr/^Perl sub died with error: kaboom/, 'and inside JS the Perl error was a catchable exception'); + is($vm->eval('fine()', 'die1b.js'), 'ok', 'VM still usable afterwards'); + + $ok = eval { $vm->eval('boom(); "not reached"', 'die2.js'); 1 }; + ok(!$ok, 'an uncaught Perl exception also dies in eval()'); + like($@, qr/^Perl sub died with error: kaboom/, 'with the Perl error'); + my $msgs = $vm->get_msgs(); + like(join('', @{ $msgs->{stderr} // [] }), qr/kaboom/, 'and the uncaught JS exception was reported like any other'); + $vm->reset_msgs(); + undef $vm; +} + +sub test_vm_destroy_after_dying_callback { + my $vm; + foreach my $round (1..3) { + undef $vm; + $vm = $CLASS->new({ save_messages => 1 }); + $vm->set('boom', sub { die "again\n" }); + eval { $vm->eval('boom()', 'die3.js') }; + is($vm->eval('1+1', 'die3b.js'), 2, "round $round: VM works after a dying callback and a VM destruction"); + } + undef $vm; + pass('destroying a VM after a dying callback does not abort'); +} + +sub test_vm_reuse_after_dying_callback { + my $vm = $CLASS->new({ save_messages => 1 }); + $vm->set('boom', sub { die "yet again\n" }); + $vm->set('big', sub { return { map { ($_ => [1..100]) } 1..200 } }); + eval { $vm->eval('boom()', 'die4.js') }; + foreach my $round (1..30) { + $vm->eval('big(); for (var k = 0; k < 2000; k++) { new Array(1000).fill(k) }', 'die4b.js'); + } + $vm->run_gc(); + pass('heavy use and GC after a dying callback do not crash'); + undef $vm; +} + +sub test_die_inside_event_loop_callback { + my $vm = $CLASS->new({ save_messages => 1 }); + $vm->set('boom', sub { die "in the loop\n" }); + $vm->eval('function render() { setTimeout(function () { boom() }, 1) }', 'loop.js'); + my $ok = eval { $vm->dispatch_function_in_event_loop('render'); 1 }; + ok(!$ok, 'a callback dying inside the event loop dies in dispatch_function_in_event_loop()'); + like($@, qr/^Perl sub died with error: in the loop/, 'with the Perl error'); + is($vm->eval('2+2', 'loop2.js'), 4, 'VM still usable afterwards'); + undef $vm; +} + +sub test_argument_errors { + my $vm = $CLASS->new({ save_messages => 1 }); + $vm->set('cb', sub { return defined $_[0] ? 'defined' : 'undef' }); + is($vm->eval('cb(function(){})', 'arg1.js'), 'undef', 'plain JS function argument converts to undef, not garbage'); + is($vm->eval('cb(cb)', 'arg2.js'), 'defined', 'Perl-backed JS function argument converts to the Perl sub'); + my $ok = eval { $vm->eval('cb(Symbol("x"))', 'arg3.js'); 1 }; + ok(!$ok, 'an unconvertible argument is an error'); + like($@, qr/Don't know how to deal with this thing/, 'reported to the Perl caller without a croak through V8'); + is($vm->eval('cb(1)', 'arg4.js'), 'defined', 'VM still usable afterwards'); + undef $vm; +} + +sub main { + use_ok($CLASS); + test_perl_die_still_reaches_the_perl_caller(); + test_vm_destroy_after_dying_callback(); + test_vm_reuse_after_dying_callback(); + test_die_inside_event_loop_callback(); + test_argument_errors(); + done_testing; + return 0; +} + +exit main();