Skip to content

Commit b2cfa35

Browse files
authored
Avoid re-using thread IDs. NFC (#15182)
I'm not clear this fixes any known issues but it certainly makes debugging pthread issues easier when each thread has a unique and non-reusaable ID. I believe its also safer then resuing the address of the pthread structure here since that can (and often is) re-used for other things or indeed other new threads (so it not unique for the life of the program). I also noticed that this field (and serveral others) are no longer needed in `struct_info_internal.json`.
1 parent b721987 commit b2cfa35

File tree

4 files changed

+9
-19
lines changed

4 files changed

+9
-19
lines changed

src/struct_info_internal.json

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,17 +9,11 @@
99
"threadStatus",
1010
"profilerBlock",
1111
"self",
12-
"tsd",
1312
"detached",
1413
"stack",
1514
"stack_size",
1615
"result",
17-
"attr",
18-
"robust_list",
19-
"tid",
20-
"canceldisable",
21-
"cancelasync",
22-
"locale"
16+
"attr"
2317
],
2418
"thread_profiler_block": [
2519
"threadStatus",

system/lib/pthread/library_pthread.c

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -860,9 +860,8 @@ void __emscripten_init_main_thread(void) {
860860
__main_pthread.self = &__main_pthread;
861861
// pthread struct robust_list head should point to itself.
862862
__main_pthread.robust_list.head = &__main_pthread.robust_list.head;
863-
864-
// Main thread ID.
865-
__main_pthread.tid = (long)&__main_pthread;
866-
863+
// Main thread ID is always 1. It can't be 0 because musl assumes
864+
// tid is always non-zero.
865+
__main_pthread.tid = 1;
867866
__main_pthread.locale = &libc.global_locale;
868867
}

system/lib/pthread/pthread_create.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,9 @@ static void init_file_lock(FILE *f) {
6666
if (f && f->lock<0) f->lock = 0;
6767
}
6868

69+
// The main thread is tid 1, and the first created thread gets tid 2.
70+
static pid_t next_tid = 2;
71+
6972
int __pthread_create(pthread_t *restrict res, const pthread_attr_t *restrict attrp, void *(*entry)(void *), void *restrict arg) {
7073
// Note on LSAN: lsan intercepts/wraps calls to pthread_create so any
7174
// allocation we we do here should be considered leaks.
@@ -92,7 +95,7 @@ int __pthread_create(pthread_t *restrict res, const pthread_attr_t *restrict att
9295
// The pthread struct has a field that points to itself - this is used as a
9396
// magic ID to detect whether the pthread_t structure is 'alive'.
9497
new->self = new;
95-
new->tid = (uintptr_t)new;
98+
new->tid = next_tid++;
9699

97100
// pthread struct robust_list head should point to itself.
98101
new->robust_list.head = &new->robust_list.head;

tests/reference_struct_info.json

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1488,19 +1488,13 @@
14881488
"pthread": {
14891489
"__size__": 224,
14901490
"attr": 100,
1491-
"cancelasync": 56,
1492-
"canceldisable": 52,
14931491
"detached": 60,
1494-
"locale": 168,
14951492
"profilerBlock": 4,
14961493
"result": 88,
1497-
"robust_list": 148,
14981494
"self": 8,
14991495
"stack": 72,
15001496
"stack_size": 76,
1501-
"threadStatus": 0,
1502-
"tid": 36,
1503-
"tsd": 96
1497+
"threadStatus": 0
15041498
},
15051499
"rlimit": {
15061500
"__size__": 16,

0 commit comments

Comments
 (0)