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

Fix #807: recv wait can deadlock on an application thread #808

Merged
merged 10 commits into from
Jul 16, 2024
Merged
Changes from 1 commit
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
Prev Previous commit
Next Next commit
Try a simpler approach
oleavr committed Jul 15, 2024

Verified

This commit was signed with the committer’s verified signature.
jcarpent Justin Carpentier
commit e7348526aa463b3aa0822fd1ffabcad1a8660abe
36 changes: 6 additions & 30 deletions bindings/gumjs/gumquickcore.c
Original file line number Diff line number Diff line change
@@ -45,7 +45,6 @@ typedef guint8 GumQuickCodeTraps;
typedef guint8 GumQuickReturnValueShape;
typedef struct _GumQuickFFIFunction GumQuickFFIFunction;
typedef struct _GumQuickCallbackContext GumQuickCallbackContext;
typedef struct _GumQuickThreadData GumQuickThreadData;

struct _GumQuickFlushCallback
{
@@ -163,11 +162,6 @@ struct _GumQuickCallbackContext
int initial_property_count;
};

struct _GumQuickThreadData
{
guint event_count_last_seen;
};

static gboolean gum_quick_core_handle_crashed_js (GumExceptionDetails * details,
gpointer user_data);

@@ -389,8 +383,6 @@ static JSValue gum_quick_value_from_ffi (JSContext * ctx,
static void gum_quick_core_setup_atoms (GumQuickCore * self);
static void gum_quick_core_teardown_atoms (GumQuickCore * self);

static GumQuickThreadData * get_gum_quick_thread_data (void);

static const JSCFunctionListEntry gumjs_root_entries[] =
{
JS_CFUNC_DEF ("_setTimeout", 0, gumjs_set_timeout),
@@ -1378,8 +1370,6 @@ static const JSCFunctionListEntry gumjs_worker_entries[] =
JS_CFUNC_DEF ("post", 0, gumjs_worker_post),
};

static GPrivate gum_quick_thread_data;

void
_gum_quick_core_init (GumQuickCore * self,
GumQuickScript * script,
@@ -2720,12 +2710,16 @@ GUMJS_DEFINE_FUNCTION (gumjs_set_incoming_message_callback)
GUMJS_DEFINE_FUNCTION (gumjs_wait_for_event)
{
GumQuickCore * self = core;
guint start_count;
GumQuickScope scope = GUM_QUICK_SCOPE_INIT (self);
GMainContext * context;
gboolean called_from_js_thread;
GumQuickThreadData * thread_data = get_gum_quick_thread_data ();
gboolean event_source_available;

g_mutex_lock (&self->event_mutex);
start_count = self->event_count;
g_mutex_unlock (&self->event_mutex);

_gum_quick_scope_perform_pending_io (self->current_scope);

_gum_quick_scope_suspend (&scope);
@@ -2735,8 +2729,7 @@ GUMJS_DEFINE_FUNCTION (gumjs_wait_for_event)

g_mutex_lock (&self->event_mutex);

while (self->event_count == thread_data->event_count_last_seen
&& self->event_source_available)
while (self->event_count == start_count && self->event_source_available)
{
if (called_from_js_thread)
{
@@ -2750,8 +2743,6 @@ GUMJS_DEFINE_FUNCTION (gumjs_wait_for_event)
}
}

thread_data->event_count_last_seen = self->event_count;

event_source_available = self->event_source_available;

g_mutex_unlock (&self->event_mutex);
@@ -5847,18 +5838,3 @@ gum_quick_core_teardown_atoms (GumQuickCore * self)

#undef GUM_TEARDOWN_ATOM
}

static GumQuickThreadData *
get_gum_quick_thread_data (void)
{
GumQuickThreadData * data = g_private_get (&gum_quick_thread_data);

if (data == NULL)
{
data = g_new0 (GumQuickThreadData, 1);
data->event_count_last_seen = 0;
g_private_set (&gum_quick_thread_data, (gpointer) data);
}

return data;
}
35 changes: 5 additions & 30 deletions bindings/gumjs/gumv8core.cpp
Original file line number Diff line number Diff line change
@@ -179,11 +179,6 @@ struct GumV8SourceMap
GumV8Core * core;
};

struct GumV8ThreadData
{
guint event_count_last_seen;
};

static gboolean gum_v8_core_handle_crashed_js (GumExceptionDetails * details,
gpointer user_data);

@@ -389,8 +384,6 @@ static gboolean gum_v8_value_to_ffi_type (GumV8Core * core,
static gboolean gum_v8_value_from_ffi_type (GumV8Core * core,
Local<Value> * svalue, const GumFFIValue * value, const ffi_type * type);

static GumV8ThreadData * get_gum_v8_thread_data ();

static const GumV8Function gumjs_global_functions[] =
{
{ "_setTimeout", gumjs_set_timeout, },
@@ -535,8 +528,6 @@ static const GumV8Function gumjs_source_map_functions[] =
{ NULL, NULL }
};

static GPrivate gum_v8_thread_data;

void
_gum_v8_core_init (GumV8Core * self,
GumV8Script * script,
@@ -1610,21 +1601,23 @@ GUMJS_DEFINE_FUNCTION (gumjs_set_incoming_message_callback)

GUMJS_DEFINE_FUNCTION (gumjs_wait_for_event)
{
g_mutex_lock (&core->event_mutex);
auto start_count = core->event_count;
g_mutex_unlock (&core->event_mutex);

gboolean event_source_available;

core->current_scope->PerformPendingIO ();

{
ScriptUnlocker unlocker (core);

auto thread_data = get_gum_v8_thread_data ();
auto context = gum_script_scheduler_get_js_context (core->scheduler);
gboolean called_from_js_thread = g_main_context_is_owner (context);

g_mutex_lock (&core->event_mutex);

while (core->event_count == thread_data->event_count_last_seen
&& core->event_source_available)
while (core->event_count == start_count && core->event_source_available)
{
if (called_from_js_thread)
{
@@ -1638,8 +1631,6 @@ GUMJS_DEFINE_FUNCTION (gumjs_wait_for_event)
}
}

thread_data->event_count_last_seen = core->event_count;

event_source_available = core->event_source_available;

g_mutex_unlock (&core->event_mutex);
@@ -4595,19 +4586,3 @@ gum_v8_value_from_ffi_type (GumV8Core * core,

return TRUE;
}

static GumV8ThreadData *
get_gum_v8_thread_data ()
{
GumV8ThreadData * data =
(GumV8ThreadData *) g_private_get (&gum_v8_thread_data);

if (data == NULL)
{
data = g_new0 (GumV8ThreadData, 1);
data->event_count_last_seen = 0;
g_private_set (&gum_v8_thread_data, (gpointer) data);
}

return data;
}