gumjs: Fix crash in Module finalizers

Our QuickJS suspend/resume patch isn't elaborate enough to support usage
from finalizers. Given that modules are created and destroyed in waves,
however, it's probably a bit on the expensive side to suspend/resume JS
execution during the destruction of each value. So to avoid both issues
we defer the unref using an idle source.

The better longer term solution will probably be to introduce a
ModuleObserver of sorts that integrates with the platform's runtime
loader and manages the lifetime of each Module. This will also allow us
to emit signals anytime a Module is added/removed, which is a feature
that a lot of agents would benefit from.

Kudos to @mrmacete for reporting.

bindings/gumjs/gumquickmodule.c

@@ -7,6 +7,7 @@
 #include "gumquickmodule.h"
 
 #include "gumquickmacros.h"
+#include "gumquickscript-priv.h"
 
 typedef struct _GumQuickMatchContext GumQuickMatchContext;
 typedef struct _GumQuickModuleFilter GumQuickModuleFilter;
@@ -28,6 +29,8 @@ struct _GumQuickModuleFilter
   GumQuickModule * parent;
 };
 
+static gboolean gum_quick_module_do_unrefs (gpointer user_data);
+
 GUMJS_DECLARE_FUNCTION (gumjs_module_load)
 GUMJS_DECLARE_FUNCTION (gumjs_module_find_global_export_by_name)
 GUMJS_DECLARE_CONSTRUCTOR (gumjs_module_construct)
@@ -130,6 +133,9 @@ _gum_quick_module_init (GumQuickModule * self,
 
   self->core = core;
 
+  g_queue_init (&self->pending_unrefs);
+  self->unref_source = NULL;
+
   _gum_quick_core_store_module_data (core, "module", self);
 
   _gum_quick_create_class (ctx, &gumjs_module_def, core, &self->module_class,
@@ -158,13 +164,78 @@ _gum_quick_module_init (GumQuickModule * self,
 void
 _gum_quick_module_dispose (GumQuickModule * self)
 {
+  g_assert (g_queue_is_empty (&self->pending_unrefs));
 }
 
 void
 _gum_quick_module_finalize (GumQuickModule * self)
 {
 }
 
+static void
+gum_quick_module_schedule_unref (GumQuickModule * self,
+                                 GObject * object)
+{
+  GumQuickCore * core = self->core;
+
+  if (_gum_quick_script_get_state (core->script) == GUM_SCRIPT_STATE_UNLOADING)
+  {
+    g_object_unref (object);
+    return;
+  }
+
+  g_queue_push_tail (&self->pending_unrefs, object);
+
+  if (self->unref_source == NULL)
+  {
+    GSource * source;
+
+    source = g_idle_source_new ();
+    g_source_set_callback (source, gum_quick_module_do_unrefs, self, NULL);
+    g_source_attach (source,
+        gum_script_scheduler_get_js_context (core->scheduler));
+    self->unref_source = source;
+
+    _gum_quick_core_pin (core);
+  }
+}
+
+static gboolean
+gum_quick_module_do_unrefs (gpointer user_data)
+{
+  GumQuickModule * self = user_data;
+  GumQuickCore * core = self->core;
+  GumQuickScope scope;
+
+  while (TRUE)
+  {
+    GObject * object;
+
+    g_rec_mutex_lock (core->mutex);
+
+    object = g_queue_pop_head (&self->pending_unrefs);
+
+    if (object == NULL)
+    {
+      g_source_unref (self->unref_source);
+      self->unref_source = NULL;
+    }
+
+    g_rec_mutex_unlock (core->mutex);
+
+    if (object == NULL)
+      break;
+
+    g_object_unref (object);
+  }
+
+  _gum_quick_scope_enter (&scope, core);
+  _gum_quick_core_unpin (core);
+  _gum_quick_scope_leave (&scope);
+
+  return G_SOURCE_REMOVE;
+}
+
 JSValue
 _gum_quick_module_new_from_handle (JSContext * ctx,
                                    GumModule * module,
@@ -253,18 +324,16 @@ GUMJS_DEFINE_CONSTRUCTOR (gumjs_module_construct)
 
 GUMJS_DEFINE_FINALIZER (gumjs_module_finalize)
 {
+  GumQuickModule * parent;
   GumModule * m;
-  GumQuickScope scope = GUM_QUICK_SCOPE_INIT (core);
 
-  m = JS_GetOpaque (val, gumjs_get_parent_module (core)->module_class);
+  parent = gumjs_get_parent_module (core);
+
+  m = JS_GetOpaque (val, parent->module_class);
   if (m == NULL)
     return;
 
-  _gum_quick_scope_suspend (&scope);
-
-  g_object_unref (m);
-
-  _gum_quick_scope_resume (&scope);
+  gum_quick_module_schedule_unref (parent, G_OBJECT (m));
 }
 
 GUMJS_DEFINE_GETTER (gumjs_module_get_name)
@@ -786,18 +855,16 @@ GUMJS_DEFINE_CONSTRUCTOR (gumjs_module_map_construct)
 
 GUMJS_DEFINE_FINALIZER (gumjs_module_map_finalize)
 {
+  GumQuickModule * parent;
   GumModuleMap * m;
-  GumQuickScope scope = GUM_QUICK_SCOPE_INIT (core);
 
-  m = JS_GetOpaque (val, gumjs_get_parent_module (core)->module_map_class);
+  parent = gumjs_get_parent_module (core);
+
+  m = JS_GetOpaque (val, parent->module_map_class);
   if (m == NULL)
     return;
 
-  _gum_quick_scope_suspend (&scope);
-
-  g_object_unref (m);
-
-  _gum_quick_scope_resume (&scope);
+  gum_quick_module_schedule_unref (parent, G_OBJECT (m));
 }
 
 GUMJS_DEFINE_GETTER (gumjs_module_map_get_handle)

bindings/gumjs/gumquickmodule.h

@@ -17,6 +17,9 @@ struct _GumQuickModule
 {
   GumQuickCore * core;
 
+  GQueue pending_unrefs;
+  GSource * unref_source;
+
   JSClassID module_class;
   JSClassID module_map_class;
 };

bindings/gumjs/gumquickscript-priv.h

@@ -13,8 +13,21 @@
 
 G_BEGIN_DECLS
 
+typedef guint GumScriptState;
 typedef struct _GumQuickWorker GumQuickWorker;
 
+enum _GumScriptState
+{
+  GUM_SCRIPT_STATE_CREATED,
+  GUM_SCRIPT_STATE_LOADING,
+  GUM_SCRIPT_STATE_LOADED,
+  GUM_SCRIPT_STATE_UNLOADING,
+  GUM_SCRIPT_STATE_UNLOADED
+};
+
+G_GNUC_INTERNAL GumScriptState _gum_quick_script_get_state (
+    GumQuickScript * self);
+
 G_GNUC_INTERNAL GumQuickWorker * _gum_quick_script_make_worker (
     GumQuickScript * self, const gchar * url, JSValue on_message);
 G_GNUC_INTERNAL GumQuickWorker * _gum_quick_worker_ref (

bindings/gumjs/gumquickscript.c

@@ -34,7 +34,6 @@
 # include "gumquickdatabase.h"
 #endif
 
-typedef guint GumScriptState;
 typedef struct _GumUnloadNotifyCallback GumUnloadNotifyCallback;
 typedef void (* GumUnloadNotifyFunc) (GumQuickScript * self,
     gpointer user_data);
@@ -96,15 +95,6 @@ enum
   PROP_BACKEND
 };
 
-enum _GumScriptState
-{
-  GUM_SCRIPT_STATE_CREATED,
-  GUM_SCRIPT_STATE_LOADING,
-  GUM_SCRIPT_STATE_LOADED,
-  GUM_SCRIPT_STATE_UNLOADING,
-  GUM_SCRIPT_STATE_UNLOADED
-};
-
 struct _GumUnloadNotifyCallback
 {
   GumUnloadNotifyFunc func;
@@ -1092,6 +1082,12 @@ gum_quick_emit_data_free (GumEmitData * d)
   g_slice_free (GumEmitData, d);
 }
 
+GumScriptState
+_gum_quick_script_get_state (GumQuickScript * self)
+{
+  return self->state;
+}
+
 GumQuickWorker *
 _gum_quick_script_make_worker (GumQuickScript * self,
                                const gchar * url,

bindings/gumjs/gumv8module.cpp

@@ -8,6 +8,7 @@
 
 #include "gumv8macros.h"
 #include "gumv8matchcontext.h"
+#include "gumv8script-priv.h"
 
 #include <gum/gum-init.h>
 #include <string.h>
@@ -71,6 +72,8 @@ struct GumV8ModuleFilter
   GumV8Module * module;
 };
 
+static gboolean gum_v8_module_do_unrefs (gpointer user_data);
+
 GUMJS_DECLARE_FUNCTION (gumjs_module_load)
 GUMJS_DECLARE_FUNCTION (gumjs_module_find_global_export_by_name)
 GUMJS_DECLARE_GETTER (gumjs_module_get_name)
@@ -183,6 +186,9 @@ _gum_v8_module_init (GumV8Module * self,
 
   self->core = core;
 
+  g_queue_init (&self->pending_unrefs);
+  self->unref_source = NULL;
+
   auto module = External::New (isolate, self);
 
   auto klass = _gum_v8_create_class ("Module", nullptr, scope, module, isolate);
@@ -245,6 +251,8 @@ _gum_v8_module_realize (GumV8Module * self)
 void
 _gum_v8_module_dispose (GumV8Module * self)
 {
+  g_assert (g_queue_is_empty (&self->pending_unrefs));
+
   g_hash_table_unref (self->values);
   self->values = NULL;
 
@@ -278,6 +286,67 @@ _gum_v8_module_finalize (GumV8Module * self)
 {
 }
 
+static void
+gum_v8_module_schedule_unref (GumV8Module * self,
+                              GObject * object)
+{
+  auto core = self->core;
+
+  if (core->script->state == GUM_SCRIPT_STATE_UNLOADING)
+  {
+    g_object_unref (object);
+    return;
+  }
+
+  g_queue_push_tail (&self->pending_unrefs, object);
+
+  if (self->unref_source == NULL)
+  {
+    auto source = g_idle_source_new ();
+    g_source_set_callback (source, gum_v8_module_do_unrefs, self, NULL);
+    g_source_attach (source,
+        gum_script_scheduler_get_js_context (core->scheduler));
+    self->unref_source = source;
+
+    _gum_v8_core_pin (core);
+  }
+}
+
+static gboolean
+gum_v8_module_do_unrefs (gpointer user_data)
+{
+  auto self = (GumV8Module *) user_data;
+  auto core = self->core;
+
+  while (TRUE)
+  {
+    GObject * object;
+    {
+      Locker locker (core->isolate);
+
+      object = G_OBJECT (g_queue_pop_head (&self->pending_unrefs));
+
+      if (object == NULL)
+      {
+        g_source_unref (self->unref_source);
+        self->unref_source = NULL;
+      }
+    }
+    if (object == NULL)
+      break;
+
+    g_object_unref (object);
+  }
+
+  {
+    ScriptScope scope (core->script);
+
+    _gum_v8_core_unpin (core);
+  }
+
+  return G_SOURCE_REMOVE;
+}
+
 GUMJS_DEFINE_FUNCTION (gumjs_module_load)
 {
   gchar * name;
@@ -723,11 +792,7 @@ _gum_v8_module_new_take_handle (GumModule * handle,
 static void
 gum_v8_module_value_free (GumV8ModuleValue * value)
 {
-  {
-    ScriptUnlocker unlocker (value->module->core);
-
-    g_object_unref (value->handle);
-  }
+  gum_v8_module_schedule_unref (value->module, G_OBJECT (value->handle));
 
   delete value->wrapper;
 
@@ -888,11 +953,7 @@ gum_v8_module_map_new (Local<Object> wrapper,
 static void
 gum_v8_module_map_free (GumV8ModuleMap * map)
 {
-  {
-    ScriptUnlocker unlocker (map->module->core);
-
-    g_object_unref (map->handle);
-  }
+  gum_v8_module_schedule_unref (map->module, G_OBJECT (map->handle));
 
   delete map->wrapper;
 

bindings/gumjs/gumv8module.h

@@ -16,6 +16,9 @@ struct GumV8Module
   GHashTable * values;
   GHashTable * maps;
 
+  GQueue pending_unrefs;
+  GSource * unref_source;
+
   v8::Global<v8::FunctionTemplate> * klass;
 
   v8::Global<v8::Object> * import_value;