Skip to content

Second attempt at fixing test_pthread_c11_threads#15187

Merged
sbc100 merged 1 commit into
mainfrom
really_fix_test_pthread_c11_threads
Sep 30, 2021
Merged

Second attempt at fixing test_pthread_c11_threads#15187
sbc100 merged 1 commit into
mainfrom
really_fix_test_pthread_c11_threads

Conversation

@sbc100
Copy link
Copy Markdown
Collaborator

@sbc100 sbc100 commented Sep 30, 2021

This time I'm pretty sure the deadlock no longer
occurs. The issue was the pthread key destcrutor
was calling printf.

Again this doesn't fix the actual issues but avoids
hitting it this test.

Fixes: #14579
See: #15186

This time I'm pretty sure the deadlock no longer
occurs.  The issue was the pthread key destcrutor
was calling printf.

Again this doesn't fix the actual issues but avoids
hitting it this test.

Fixes: #14579
See: #15186
@sbc100 sbc100 force-pushed the really_fix_test_pthread_c11_threads branch from f68453b to 1ce93d4 Compare September 30, 2021 15:06
// exits. This means we can't use `thread_main` below because
// the destructor to the `tss_t key` writes to stdout.
int thread_main_detached(void* arg) {
printf("in thread_main_detached %p\n", (void*)thrd_current());
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This writes to stdout here - isn't that as bad as the other print we are avoiding?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This happens before the cnd_signal below.. the main thread waits for that signal before exiting so anything before that is fine.

@sbc100 sbc100 enabled auto-merge (squash) September 30, 2021 15:39
@sbc100
Copy link
Copy Markdown
Collaborator Author

sbc100 commented Sep 30, 2021

I ran this test continuously for over an hour (while running the whole test suite in the background to cause contention for cpu cores) and i didn't timeout at all.. so I'm pretty sure this test is fixed now.

@sbc100 sbc100 merged commit b721987 into main Sep 30, 2021
@sbc100 sbc100 deleted the really_fix_test_pthread_c11_threads branch September 30, 2021 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

wasm2js1.test_pthread_c11_threads is showing flakiness

3 participants