Fixing the segfault when running running perl 5.42

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

V8Context.cc

@@ -29,6 +29,27 @@
 int V8Context::instance_count = 0;
 std::unique_ptr<v8::Platform> 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()

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);
     }
 }
 

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);
     }
 }
 

pl_util.cc

@@ -1,7 +1,41 @@
+#include <stdarg.h>
 #include <stdio.h>
 #include <sys/time.h>
 #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)

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