Skip to content

Commit 0a78042

Browse files
authored
Fix EM_CALLBACK_THREAD_CONTEXT_CALLING_THREAD (#19691)
I think this has probably been broken for a long time. Given that nobody has complained we could probably just remove this feature, but on the other hand the fix wasn't too hard. Confirmed that interactive.test_html5_callback_on_two_threads fails before this change and passed afterwards. Fixes: #19690
1 parent 3ce924f commit 0a78042

File tree

5 files changed

+32
-15
lines changed

5 files changed

+32
-15
lines changed

src/library_html5.js

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -210,9 +210,23 @@ var LibraryHTML5 = {
210210
#if PTHREADS
211211
getTargetThreadForEventCallback: function(targetThread) {
212212
switch (targetThread) {
213-
case {{{ cDefs.EM_CALLBACK_THREAD_CONTEXT_MAIN_RUNTIME_THREAD }}}: return 0; // The event callback for the current event should be called on the main browser thread. (0 == don't proxy)
214-
case {{{ cDefs.EM_CALLBACK_THREAD_CONTEXT_CALLING_THREAD }}}: return PThread.currentProxiedOperationCallerThread; // The event callback for the current event should be backproxied to the thread that is registering the event.
215-
default: return targetThread; // The event callback for the current event should be proxied to the given specific thread.
213+
case {{{ cDefs.EM_CALLBACK_THREAD_CONTEXT_MAIN_RUNTIME_THREAD }}}:
214+
// The event callback for the current event should be called on the
215+
// main browser thread. (0 == don't proxy)
216+
return 0;
217+
case {{{ cDefs.EM_CALLBACK_THREAD_CONTEXT_CALLING_THREAD }}}:
218+
// The event callback for the current event should be backproxied to
219+
// the thread that is registering the event.
220+
#if ASSERTIONS
221+
// If we get here PThread.currentProxiedOperationCallerThread should
222+
// be set to the calling thread.
223+
assert(PThread.currentProxiedOperationCallerThread);
224+
#endif
225+
return PThread.currentProxiedOperationCallerThread;
226+
default:
227+
// The event callback for the current event should be proxied to the
228+
// given specific thread.
229+
return targetThread;
216230
}
217231
},
218232
#endif

src/library_pthread.js

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -258,11 +258,6 @@ var LibraryPThread = {
258258
worker.onmessage = (e) => {
259259
var d = e['data'];
260260
var cmd = d['cmd'];
261-
// Sometimes we need to backproxy events to the calling thread (e.g.
262-
// HTML5 DOM events handlers such as
263-
// emscripten_set_mousemove_callback()), so keep track in a globally
264-
// accessible variable about the thread that initiated the proxying.
265-
if (worker.pthread_ptr) PThread.currentProxiedOperationCallerThread = worker.pthread_ptr;
266261

267262
// If this message is intended to a recipient that is not the main thread, forward it to the target thread.
268263
if (d['targetThread'] && d['targetThread'] != _pthread_self()) {
@@ -272,7 +267,6 @@ var LibraryPThread = {
272267
} else {
273268
err('Internal error! Worker sent a message "' + cmd + '" to target pthread ' + d['targetThread'] + ', but that thread no longer exists!');
274269
}
275-
PThread.currentProxiedOperationCallerThread = undefined;
276270
return;
277271
}
278272

@@ -316,7 +310,6 @@ var LibraryPThread = {
316310
// recognized commands:
317311
err("worker sent an unknown command " + cmd);
318312
}
319-
PThread.currentProxiedOperationCallerThread = undefined;
320313
};
321314

322315
worker.onerror = (e) => {
@@ -1014,7 +1007,12 @@ var LibraryPThread = {
10141007
emscripten_receive_on_main_thread_js__deps: [
10151008
'$proxyToMainThread',
10161009
'$emscripten_receive_on_main_thread_js_callArgs'],
1017-
emscripten_receive_on_main_thread_js: function(index, numCallArgs, args) {
1010+
emscripten_receive_on_main_thread_js: function(index, callingThread, numCallArgs, args) {
1011+
// Sometimes we need to backproxy events to the calling thread (e.g.
1012+
// HTML5 DOM events handlers such as
1013+
// emscripten_set_mousemove_callback()), so keep track in a globally
1014+
// accessible variable about the thread that initiated the proxying.
1015+
PThread.currentProxiedOperationCallerThread = callingThread;
10181016
#if WASM_BIGINT
10191017
numCallArgs /= 2;
10201018
#endif

src/library_sigs.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -698,7 +698,7 @@ sigs = {
698698
emscripten_promise_resolve__sig: 'vpip',
699699
emscripten_promise_then__sig: 'ppppp',
700700
emscripten_random__sig: 'f',
701-
emscripten_receive_on_main_thread_js__sig: 'diip',
701+
emscripten_receive_on_main_thread_js__sig: 'dipip',
702702
emscripten_request_animation_frame__sig: 'ipp',
703703
emscripten_request_animation_frame_loop__sig: 'vpp',
704704
emscripten_request_fullscreen__sig: 'ipi',

system/lib/pthread/library_pthread.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -188,7 +188,7 @@ static void _do_call(void* arg) {
188188
break;
189189
case EM_PROXIED_JS_FUNCTION:
190190
q->returnValue.d =
191-
emscripten_receive_on_main_thread_js((intptr_t)q->functionPtr, q->args[0].i, &q->args[1].d);
191+
emscripten_receive_on_main_thread_js((intptr_t)q->functionPtr, q->callingThread, q->args[0].i, &q->args[1].d);
192192
break;
193193
case EM_FUNC_SIG_V:
194194
((em_func_v)q->functionPtr)();
@@ -430,10 +430,11 @@ double _emscripten_run_in_main_runtime_thread_js(int index, int num_args, int64_
430430
} else {
431431
c = em_queued_call_malloc();
432432
}
433-
c->calleeDelete = 1-sync;
433+
c->calleeDelete = !sync;
434434
c->functionEnum = EM_PROXIED_JS_FUNCTION;
435435
// Index not needed to ever be more than 32-bit.
436436
c->functionPtr = (void*)(intptr_t)index;
437+
c->callingThread = pthread_self();
437438
assert(num_args+1 <= EM_QUEUED_JS_CALL_MAX_ARGS);
438439
// The types are only known at runtime in these calls, so we store values that
439440
// must be able to contain any valid JS value, including a 64-bit BigInt if

system/lib/pthread/threading_internal.h

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ typedef struct em_queued_call {
3333
em_variant_val args[EM_QUEUED_JS_CALL_MAX_ARGS];
3434
em_variant_val returnValue;
3535

36+
// Sets the PThread.currentProxiedOperationCallerThread global for the
37+
// duration of the proxied call.
38+
pthread_t callingThread;
39+
3640
// An optional pointer to a secondary data block that should be free()d when
3741
// this queued call is freed.
3842
void *satelliteData;
@@ -163,4 +167,4 @@ int __pthread_create_js(struct __pthread *thread, const pthread_attr_t *attr, vo
163167
int _emscripten_default_pthread_stack_size();
164168
void __set_thread_state(pthread_t ptr, int is_main, int is_runtime, int can_block);
165169

166-
double emscripten_receive_on_main_thread_js(int functionIndex, int numCallArgs, double* args);
170+
double emscripten_receive_on_main_thread_js(int functionIndex, pthread_t callingThread, int numCallArgs, double* args);

0 commit comments

Comments
 (0)