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
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
8 changes: 6 additions & 2 deletions bindings/gumjs/gumquickcore.c
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* Copyright (C) 2020-2022 Francesco Tamagni <[email protected]>
* Copyright (C) 2020 Marcus Mengs <[email protected]>
* Copyright (C) 2021 Abdelrahman Eid <[email protected]>
* Copyright (C) 2024 Simon Zuckerbraun <[email protected]>
*
* Licence: wxWindows Library Licence, Version 3.1
*/
Expand Down Expand Up @@ -2710,12 +2711,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;
guint start_count;
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);
Expand All @@ -2725,7 +2730,6 @@ GUMJS_DEFINE_FUNCTION (gumjs_wait_for_event)

g_mutex_lock (&self->event_mutex);

start_count = self->event_count;
while (self->event_count == start_count && self->event_source_available)
{
if (called_from_js_thread)
Expand Down
6 changes: 5 additions & 1 deletion bindings/gumjs/gumv8core.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* Copyright (C) 2020-2022 Francesco Tamagni <[email protected]>
* Copyright (C) 2020 Marcus Mengs <[email protected]>
* Copyright (C) 2021 Abdelrahman Eid <[email protected]>
* Copyright (C) 2024 Simon Zuckerbraun <[email protected]>
*
* Licence: wxWindows Library Licence, Version 3.1
*/
Expand Down Expand Up @@ -1601,6 +1602,10 @@ 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 ();
Expand All @@ -1613,7 +1618,6 @@ GUMJS_DEFINE_FUNCTION (gumjs_wait_for_event)

g_mutex_lock (&core->event_mutex);

auto start_count = core->event_count;
while (core->event_count == start_count && core->event_source_available)
{
if (called_from_js_thread)
Expand Down
74 changes: 74 additions & 0 deletions tests/gumjs/script.c
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
* Copyright (C) 2023 Grant Douglas <[email protected]>
* Copyright (C) 2024 Hillel Pinto <[email protected]>
* Copyright (C) 2024 Håvard Sørbø <[email protected]>
* Copyright (C) 2024 Simon Zuckerbraun <[email protected]>
*
* Licence: wxWindows Library Licence, Version 3.1
*/
Expand All @@ -24,6 +25,7 @@ TESTLIST_BEGIN (script)
TESTENTRY (recv_may_specify_desired_message_type)
TESTENTRY (recv_can_be_waited_for_from_an_application_thread)
TESTENTRY (recv_can_be_waited_for_from_two_application_threads)
TESTENTRY (recv_wait_in_an_application_thread_should_not_deadlock)
TESTENTRY (recv_can_be_waited_for_from_our_js_thread)
TESTENTRY (recv_wait_in_an_application_thread_should_throw_on_unload)
TESTENTRY (recv_wait_in_our_js_thread_should_throw_on_unload)
Expand Down Expand Up @@ -6125,6 +6127,78 @@ TESTCASE (message_can_be_received)
EXPECT_SEND_MESSAGE_WITH ("\"pong\"");
}

TESTCASE (recv_wait_in_an_application_thread_should_not_deadlock)
{
GThread * worker_thread;
GumInvokeTargetContext ctx;
guint i;

if (!g_test_slow ())
{
g_print ("<skipping, run in slow mode> ");
return;
}

COMPILE_AND_LOAD_SCRIPT (
"Interceptor.replace(" GUM_PTR_CONST ", new NativeCallback(arg => {"
" let timeToRecv;"
" let shouldExit = false;"
" while (true) {"
" recv(message => {"
" if (message.type === 'stop')"
" shouldExit = true;"
" else if (message.type === 'wait-until')"
" timeToRecv = message.time;"
" else"
" throw new Error(`unexpected message: ${message.type}`);"
" }).wait();"
" if (shouldExit)"
" return 0;"
" while (Date.now() < timeToRecv) {}"
" recv(message => {"
" if (message.type !== 'ping')"
" throw new Error(`unexpected message: ${message.type}`);"
" }).wait();"
" send('pong');"
" }"
"}, 'int', ['int']));", target_function_int);

ctx.script = fixture->script;
ctx.repeat_duration = 0;
ctx.started = 0;
ctx.finished = 0;
worker_thread = g_thread_new ("script-test-worker-thread",
invoke_target_function_int_worker, &ctx);
while (ctx.started == 0)
g_usleep (G_USEC_PER_SEC / 200);

for (i = 0; i != 100; i++)
{
gint64 time_now, time_to_schedule_recv, time_to_post;
gchar * msg;

time_now = g_get_real_time ();
time_to_schedule_recv = time_now - (time_now % (20 * 1000)) + (50 * 1000);
time_to_post = time_to_schedule_recv + i;

msg = g_strdup_printf (
"{\"type\":\"wait-until\",\"time\":%" G_GINT64_FORMAT "}",
time_to_schedule_recv / 1000);
POST_MESSAGE (msg);
g_free (msg);

while (g_get_real_time () < time_to_post)
;
POST_MESSAGE ("{\"type\":\"ping\"}");
EXPECT_SEND_MESSAGE_WITH ("\"pong\"");
}

POST_MESSAGE ("{\"type\":\"stop\"}");

g_thread_join (worker_thread);
g_assert_cmpint (ctx.finished, ==, 1);
}

TESTCASE (message_can_be_received_with_data)
{
const guint8 data_to_send[2] = { 0x13, 0x37 };
Expand Down
Loading