Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

gumjs: Fix crash in Module finalizers #987

Merged
merged 2 commits into from
Jan 13, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
98 changes: 84 additions & 14 deletions bindings/gumjs/gumquickmodule.c
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
#include "gumquickmodule.h"

#include "gumquickmacros.h"
#include "gumquickscript-priv.h"

typedef struct _GumQuickMatchContext GumQuickMatchContext;
typedef struct _GumQuickModuleFilter GumQuickModuleFilter;
Expand All @@ -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)
Expand Down Expand Up @@ -130,6 +133,9 @@ _gum_quick_module_init (GumQuickModule * self,

self->core = core;

self->pending_unrefs = NULL;
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,
Expand Down Expand Up @@ -158,13 +164,81 @@ _gum_quick_module_init (GumQuickModule * self,
void
_gum_quick_module_dispose (GumQuickModule * self)
{
g_assert (self->pending_unrefs == NULL);
}

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

if (self->pending_unrefs == NULL)
self->pending_unrefs = g_ptr_array_new_with_free_func (g_object_unref);

g_ptr_array_add (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)
{
GPtrArray * pending;

g_rec_mutex_lock (core->mutex);

pending = g_steal_pointer (&self->pending_unrefs);

if (pending == NULL)
{
g_source_unref (self->unref_source);
self->unref_source = NULL;
}

g_rec_mutex_unlock (core->mutex);

if (pending == NULL)
break;

g_ptr_array_unref (pending);
}

_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,
Expand Down Expand Up @@ -253,18 +327,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)
Expand Down Expand Up @@ -786,18 +858,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)
Expand Down
3 changes: 3 additions & 0 deletions bindings/gumjs/gumquickmodule.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ struct _GumQuickModule
{
GumQuickCore * core;

GPtrArray * pending_unrefs;
GSource * unref_source;

JSClassID module_class;
JSClassID module_map_class;
};
Expand Down
13 changes: 13 additions & 0 deletions bindings/gumjs/gumquickscript-priv.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down
16 changes: 6 additions & 10 deletions bindings/gumjs/gumquickscript.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
# include "gumquickdatabase.h"
#endif

typedef guint GumScriptState;
typedef struct _GumUnloadNotifyCallback GumUnloadNotifyCallback;
typedef void (* GumUnloadNotifyFunc) (GumQuickScript * self,
gpointer user_data);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
85 changes: 75 additions & 10 deletions bindings/gumjs/gumv8module.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

#include "gumv8macros.h"
#include "gumv8matchcontext.h"
#include "gumv8script-priv.h"

#include <gum/gum-init.h>
#include <string.h>
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -183,6 +186,9 @@ _gum_v8_module_init (GumV8Module * self,

self->core = core;

self->pending_unrefs = NULL;
self->unref_source = NULL;

auto module = External::New (isolate, self);

auto klass = _gum_v8_create_class ("Module", nullptr, scope, module, isolate);
Expand Down Expand Up @@ -245,6 +251,8 @@ _gum_v8_module_realize (GumV8Module * self)
void
_gum_v8_module_dispose (GumV8Module * self)
{
g_assert (self->pending_unrefs == NULL);

g_hash_table_unref (self->values);
self->values = NULL;

Expand Down Expand Up @@ -278,6 +286,71 @@ _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;
}

if (self->pending_unrefs == NULL)
self->pending_unrefs = g_ptr_array_new_with_free_func (g_object_unref);

g_ptr_array_add (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)
{
GPtrArray * pending;
{
Locker locker (core->isolate);

pending = (GPtrArray *) g_steal_pointer (&self->pending_unrefs);

if (pending == NULL)
{
g_source_unref (self->unref_source);
self->unref_source = NULL;
}
}

if (pending == NULL)
break;

g_ptr_array_unref (pending);
}

{
ScriptScope scope (core->script);

_gum_v8_core_unpin (core);
}

return G_SOURCE_REMOVE;
}

GUMJS_DEFINE_FUNCTION (gumjs_module_load)
{
gchar * name;
Expand Down Expand Up @@ -723,11 +796,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;

Expand Down Expand Up @@ -888,11 +957,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;

Expand Down
3 changes: 3 additions & 0 deletions bindings/gumjs/gumv8module.h
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ struct GumV8Module
GHashTable * values;
GHashTable * maps;

GPtrArray * pending_unrefs;
GSource * unref_source;

v8::Global<v8::FunctionTemplate> * klass;

v8::Global<v8::Object> * import_value;
Expand Down
Loading
Loading