Skip to content

Commit

Permalink
[wasm64] Fix _emscripten_run_callback_on_thread under wasm64 (#21852)
Browse files Browse the repository at this point in the history
The only test we have for this was in `test_interactive.py`.  I ran it
locally to ensure it now works.

Fixes: #21851
  • Loading branch information
sbc100 committed Apr 29, 2024
1 parent 7f369e1 commit 0649692
Show file tree
Hide file tree
Showing 7 changed files with 34 additions and 27 deletions.
1 change: 1 addition & 0 deletions system/include/emscripten/threading_legacy.h
Expand Up @@ -114,6 +114,7 @@ typedef struct em_queued_call em_queued_call;
#define EM_FUNC_SIG_II (EM_FUNC_SIG_RETURN_VALUE_I | EM_FUNC_SIG_WITH_N_PARAMETERS(1) | EM_FUNC_SIG_SET_PARAM(0, EM_FUNC_SIG_PARAM_I))
#define EM_FUNC_SIG_III (EM_FUNC_SIG_RETURN_VALUE_I | EM_FUNC_SIG_WITH_N_PARAMETERS(2) | EM_FUNC_SIG_SET_PARAM(0, EM_FUNC_SIG_PARAM_I) | EM_FUNC_SIG_SET_PARAM(1, EM_FUNC_SIG_PARAM_I))
#define EM_FUNC_SIG_IIII (EM_FUNC_SIG_RETURN_VALUE_I | EM_FUNC_SIG_WITH_N_PARAMETERS(3) | EM_FUNC_SIG_SET_PARAM(0, EM_FUNC_SIG_PARAM_I) | EM_FUNC_SIG_SET_PARAM(1, EM_FUNC_SIG_PARAM_I) | EM_FUNC_SIG_SET_PARAM(2, EM_FUNC_SIG_PARAM_I))
#define EM_FUNC_SIG_IIPP (EM_FUNC_SIG_RETURN_VALUE_I | EM_FUNC_SIG_WITH_N_PARAMETERS(3) | EM_FUNC_SIG_SET_PARAM(0, EM_FUNC_SIG_PARAM_I) | EM_FUNC_SIG_SET_PARAM(1, EM_FUNC_SIG_PARAM_P) | EM_FUNC_SIG_SET_PARAM(2, EM_FUNC_SIG_PARAM_P))
#define EM_FUNC_SIG_IIIII (EM_FUNC_SIG_RETURN_VALUE_I | EM_FUNC_SIG_WITH_N_PARAMETERS(4) | EM_FUNC_SIG_SET_PARAM(0, EM_FUNC_SIG_PARAM_I) | EM_FUNC_SIG_SET_PARAM(1, EM_FUNC_SIG_PARAM_I) | EM_FUNC_SIG_SET_PARAM(2, EM_FUNC_SIG_PARAM_I) | EM_FUNC_SIG_SET_PARAM(3, EM_FUNC_SIG_PARAM_I))
#define EM_FUNC_SIG_IIIIII (EM_FUNC_SIG_RETURN_VALUE_I | EM_FUNC_SIG_WITH_N_PARAMETERS(5) | EM_FUNC_SIG_SET_PARAM(0, EM_FUNC_SIG_PARAM_I) | EM_FUNC_SIG_SET_PARAM(1, EM_FUNC_SIG_PARAM_I) | EM_FUNC_SIG_SET_PARAM(2, EM_FUNC_SIG_PARAM_I) | EM_FUNC_SIG_SET_PARAM(3, EM_FUNC_SIG_PARAM_I) | EM_FUNC_SIG_SET_PARAM(4, EM_FUNC_SIG_PARAM_I))
#define EM_FUNC_SIG_IIIIIII (EM_FUNC_SIG_RETURN_VALUE_I | EM_FUNC_SIG_WITH_N_PARAMETERS(6) | EM_FUNC_SIG_SET_PARAM(0, EM_FUNC_SIG_PARAM_I) | EM_FUNC_SIG_SET_PARAM(1, EM_FUNC_SIG_PARAM_I) | EM_FUNC_SIG_SET_PARAM(2, EM_FUNC_SIG_PARAM_I) | EM_FUNC_SIG_SET_PARAM(3, EM_FUNC_SIG_PARAM_I) | EM_FUNC_SIG_SET_PARAM(4, EM_FUNC_SIG_PARAM_I) | EM_FUNC_SIG_SET_PARAM(5, EM_FUNC_SIG_PARAM_I))
Expand Down
4 changes: 4 additions & 0 deletions system/lib/pthread/proxying_legacy.c
Expand Up @@ -89,6 +89,7 @@ typedef int (*em_func_ij)(uint64_t);
typedef uint64_t (*em_func_ji)(int);
typedef int (*em_func_ijj)(uint64_t, uint64_t);
typedef int (*em_func_iij)(int, uint64_t);
typedef int (*em_func_iijj)(int, uint64_t, uint64_t);
typedef uint64_t (*em_func_jjj)(uint64_t, uint64_t);
typedef void (*em_func_vij)(int, uint64_t);
typedef void (*em_func_viij)(int, int, uint64_t);
Expand Down Expand Up @@ -249,6 +250,9 @@ static void _do_call(void* arg) {
case EM_FUNC_SIG_IP:
q->returnValue.i = ((em_func_ij)q->functionPtr)(q->args[0].j);
break;
case EM_FUNC_SIG_IIPP:
q->returnValue.i = ((em_func_iijj)q->functionPtr)(q->args[0].i, q->args[1].j, q->args[2].j);
break;
case EM_FUNC_SIG_PI:
q->returnValue.j = ((em_func_ji)q->functionPtr)(q->args[0].i);
break;
Expand Down
4 changes: 1 addition & 3 deletions test/emscripten_hide_mouse.c
Expand Up @@ -5,9 +5,7 @@

EM_BOOL mouse_callback(int eventType, const EmscriptenMouseEvent *e, void *userData) {
printf("Mouse click on canvas.\n");
#ifdef REPORT_RESULT
REPORT_RESULT(0);
#endif
emscripten_force_exit(0);
return 0;
}

Expand Down
27 changes: 10 additions & 17 deletions test/html5_event_callback_in_two_threads.c
Expand Up @@ -3,24 +3,17 @@
#include <emscripten/threading.h>
#include <emscripten/key_codes.h>
#include <stdio.h>
#include <stdbool.h>
#include <stdlib.h>
#include <memory.h>
#include <assert.h>

pthread_t application_thread_id = 0;
pthread_t main_runtime_thread_id = 0;

volatile int saw_keydown_event_on_enter_key_on_application_thread = 0;
volatile int saw_keydown_event_on_enter_key_on_main_runtime_thread = 0;
volatile int saw_keypress_event_on_enter_key = 0;

void ReportResult(int code) {
printf("Test finished with code: %d\n", code);
#ifdef REPORT_RESULT
REPORT_RESULT(code);
#endif
exit(code);
}
_Atomic int saw_keydown_event_on_enter_key_on_application_thread = 0;
_Atomic int saw_keydown_event_on_enter_key_on_main_runtime_thread = 0;
_Atomic int saw_keypress_event_on_enter_key = 0;

EM_BOOL keydown_callback_on_application_thread(int eventType, const EmscriptenKeyboardEvent *e, void *userData) {
int dom_pk_code = emscripten_compute_dom_pk_code(e->code);
Expand All @@ -41,7 +34,7 @@ EM_BOOL keydown_callback_on_main_runtime_thread(int eventType, const EmscriptenK
#if __EMSCRIPTEN_PTHREADS__
EmscriptenKeyboardEvent *duplicatedEventStruct = malloc(sizeof(*e));
memcpy(duplicatedEventStruct, e, sizeof(*e));
emscripten_dispatch_to_thread(application_thread_id, EM_FUNC_SIG_IIII, keydown_callback_on_application_thread, duplicatedEventStruct, eventType, duplicatedEventStruct, userData);
emscripten_dispatch_to_thread(application_thread_id, EM_FUNC_SIG_IIPP, keydown_callback_on_application_thread, duplicatedEventStruct, eventType, duplicatedEventStruct, userData);
#else
keydown_callback_on_application_thread(eventType, e, userData);
#endif
Expand All @@ -63,7 +56,7 @@ EM_BOOL keypress_callback_on_application_thread(int eventType, const EmscriptenK
if (dom_pk_code == DOM_PK_ENTER) {
saw_keypress_event_on_enter_key = 1;
printf("Test failed! KeyPress event came through even though it was suppressed in KeyDown handler!\n");
ReportResult(12345); // FAIL
assert(false);
}
return 0;
}
Expand All @@ -76,18 +69,18 @@ EM_BOOL keyup_callback_on_application_thread(int eventType, const EmscriptenKeyb
if (dom_pk_code == DOM_PK_ENTER) {
if (!saw_keydown_event_on_enter_key_on_application_thread) {
printf("Test failed! KeyUp event came through, but a KeyDown event should have first been processed on the application thread!\n");
ReportResult(12346); // FAIL
assert(false);
}
if (!saw_keydown_event_on_enter_key_on_main_runtime_thread) {
printf("Test failed! KeyUp event came through, but a KeyDown event should have first been processed on the main runtime thread!\n");
ReportResult(12347); // FAIL
assert(false);
}
if (saw_keypress_event_on_enter_key) {
printf("Test failed! KeyUp event came through, but a KeyPress event was first seen, suppressing it did not work!\n");
ReportResult(12348); // FAIL
assert(false);
}
printf("Test passed!\n");
ReportResult(1); // PASS
exit(0);
}
return 0;
}
Expand Down
2 changes: 0 additions & 2 deletions test/test_browser.py
Expand Up @@ -2571,8 +2571,6 @@ def test_html5_core(self, opts):
if self.is_wasm64():
if '-sMIN_CHROME_VERSION=0' in opts:
self.skipTest('wasm64 does not support older browsers')
if '-sPROXY_TO_PTHREAD' in opts:
self.skipTest('_emscripten_set_keypress_callback_on_thread broken under wasm64')
if '-sHTML5_SUPPORT_DEFERRING_USER_SENSITIVE_REQUESTS=0' in opts:
# In this mode an exception can be thrown by the browser, and we don't
# want the test to fail in that case so we override the error handling.
Expand Down
22 changes: 17 additions & 5 deletions test/test_interactive.py
Expand Up @@ -247,12 +247,16 @@ def test_html5_callbacks_on_calling_thread(self):
})
def test_html5_event_callback_in_two_threads(self, args):
# TODO: Make this automatic by injecting enter key press in e.g. shell html file.
self.btest('html5_event_callback_in_two_threads.c', expected='1', args=args)
self.btest_exit('html5_event_callback_in_two_threads.c', args=args)

# Test that emscripten_hide_mouse() is callable from pthreads (and proxies to main thread to obtain the proper window.devicePixelRatio value).
def test_emscripten_hide_mouse(self):
for args in [[], ['-pthread']]:
self.btest('emscripten_hide_mouse.c', expected='0', args=args)
# Test that emscripten_hide_mouse() is callable from pthreads (and proxies to main
# thread to obtain the proper window.devicePixelRatio value).
@parameterized({
'': ([],),
'threads': (['-pthread'],),
})
def test_emscripten_hide_mouse(self, args):
self.btest('emscripten_hide_mouse.c', expected='0', args=args)

# Tests that WebGL can be run on another thread after first having run it on one thread (and that thread has exited). The intent of this is to stress graceful deinit semantics, so that it is not possible to "taint" a Canvas
# to a bad state after a rendering thread in a program quits and restarts. (perhaps e.g. between level loads, or subsystem loads/restarts or something like that)
Expand Down Expand Up @@ -283,3 +287,11 @@ def test_audio_worklet_tone_generator(self):
# Tests that AUDIO_WORKLET+MINIMAL_RUNTIME+MODULARIZE combination works together.
def test_audio_worklet_modularize(self):
self.btest('webaudio/audioworklet.c', expected='0', args=['-sAUDIO_WORKLET', '-sWASM_WORKERS', '-sMINIMAL_RUNTIME', '-sMODULARIZE'])


class interactive64(interactive):
def setUp(self):
super().setUp()
self.set_setting('MEMORY64')
self.emcc_args.append('-Wno-experimental')
self.require_wasm64()
1 change: 1 addition & 0 deletions tools/emscripten.py
Expand Up @@ -1076,6 +1076,7 @@ def create_pointer_conversion_wrappers(metadata):
'emscripten_main_runtime_thread_id': 'p',
'_emscripten_set_offscreencanvas_size_on_thread': '_pp__',
'fileno': '_p',
'_emscripten_run_callback_on_thread': '_pp_pp',
}

for function in settings.SIGNATURE_CONVERSIONS:
Expand Down

0 comments on commit 0649692

Please sign in to comment.