Skip to content

Commit 6abf9a3

Browse files
authored
Never try to run proxied work without a live runtime (#16510)
We were previously running `emscripten_proxy_execute_queue` from a JS callback whenever a thread proxied to itself, even if the thread runtime had died by the time the callback was received. In practice this never resulted in any work running because the queue was not able to find the task queue for the thread without the old `pthread_self` value, but it was still inconsistent with the behavior when proxying to separate threads, which does not result in a call to `emscripten_proxy_execute_queue`. Making the behavior more consistent lets us insert a new assertion that `pthread_self()` is meaningful and is more robust to future changes to the proxying mechanism. Also improve the documentation to point out that work will be dropped if the target thread dies with the work unfinished.
1 parent 52cefe8 commit 6abf9a3

File tree

6 files changed

+77
-1
lines changed

6 files changed

+77
-1
lines changed

src/library_pthread.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1051,7 +1051,11 @@ var LibraryPThread = {
10511051

10521052
_emscripten_notify_proxying_queue: function(targetThreadId, currThreadId, mainThreadId, queue) {
10531053
if (targetThreadId == currThreadId) {
1054-
setTimeout(function() { _emscripten_proxy_execute_queue(queue); });
1054+
setTimeout(() => {
1055+
if (_pthread_self()) {
1056+
_emscripten_proxy_execute_queue(queue);
1057+
}
1058+
});
10551059
} else if (ENVIRONMENT_IS_PTHREAD) {
10561060
postMessage({'targetThread' : targetThreadId, 'cmd' : 'processProxyingQueue', 'queue' : queue});
10571061
} else {

system/lib/pthread/proxying.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,7 @@ static task_queue* get_or_add_tasks_for_thread(em_proxying_queue* q,
206206
EMSCRIPTEN_KEEPALIVE
207207
void emscripten_proxy_execute_queue(em_proxying_queue* q) {
208208
assert(q != NULL);
209+
assert(pthread_self());
209210

210211
// Recursion guard to avoid infinite recursion when we arrive here from the
211212
// pthread_lock call below that executes the system queue. The per-task_queue

system/lib/pthread/proxying.h

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,11 @@ extern "C" {
1818
// asynchronously or synchronously proxied from other threads. When work is
1919
// proxied to a queue on a particular thread, that thread is notified to start
2020
// processing work from that queue if it is not already doing so.
21+
//
22+
// Proxied work can only be completed on live thread runtimes, so users must
23+
// ensure either that all proxied work is completed before a thread exits or
24+
// that the thread exits with a live runtime, e.g. via
25+
// `emscripten_exit_with_live_runtime` to avoid dropped work.
2126
typedef struct em_proxying_queue em_proxying_queue;
2227

2328
// Create and destroy proxying queues.
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
#include <assert.h>
2+
#include <emscripten/console.h>
3+
#include <pthread.h>
4+
#include <stdbool.h>
5+
#include <unistd.h>
6+
7+
#include "proxying.h"
8+
9+
em_proxying_queue* queue;
10+
11+
_Atomic int exploded = 0;
12+
13+
void explode(void* arg) {
14+
exploded = 1;
15+
assert(false);
16+
}
17+
18+
void* proxy_to_self(void* arg) {
19+
emscripten_proxy_async(queue, pthread_self(), explode, NULL);
20+
return NULL;
21+
}
22+
23+
void* do_nothing(void* arg) {
24+
return NULL;
25+
}
26+
27+
int main() {
28+
emscripten_console_log("start");
29+
queue = em_proxying_queue_create();
30+
assert(queue);
31+
32+
// Check that proxying to a thread that exits without a live runtime causes
33+
// the work to be dropped without other errors.
34+
pthread_t worker;
35+
pthread_create(&worker, NULL, do_nothing, NULL);
36+
emscripten_proxy_async(queue, worker, explode, NULL);
37+
38+
// Check that a thread proxying to itself but exiting without a live runtime
39+
// causes the work to be dropped without other errors.
40+
pthread_t self_proxier;
41+
pthread_create(&self_proxier, NULL, proxy_to_self, NULL);
42+
43+
pthread_join(worker, NULL);
44+
pthread_join(self_proxier, NULL);
45+
46+
// Wait a bit (50 ms) to see if the `assert(pthread_self())` in
47+
// emscripten_proxy_execute_queue fires or if we explode.
48+
struct timespec time = {
49+
.tv_sec = 0,
50+
.tv_nsec = 50 * 1000 * 1000,
51+
};
52+
nanosleep(&time, NULL);
53+
54+
assert(!exploded);
55+
emscripten_console_log("done");
56+
}
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
start
2+
done

tests/test_core.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2578,6 +2578,14 @@ def test_pthread_proxying(self):
25782578
self.do_run_in_out_file_test('pthread/test_pthread_proxying.c',
25792579
emcc_args=args, interleaved_output=False)
25802580

2581+
@node_pthreads
2582+
def test_pthread_proxying_dropped_work(self):
2583+
self.set_setting('EXIT_RUNTIME')
2584+
self.set_setting('PTHREAD_POOL_SIZE=2')
2585+
args = [f'-I{path_from_root("system/lib/pthread")}']
2586+
self.do_run_in_out_file_test('pthread/test_pthread_proxying_dropped_work.c',
2587+
emcc_args=args)
2588+
25812589
@node_pthreads
25822590
def test_pthread_dispatch_after_exit(self):
25832591
self.set_setting('EXIT_RUNTIME')

0 commit comments

Comments
 (0)