Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not run any cleanup if the program is exiting anyway #2372

Merged
merged 1 commit into from
Jan 21, 2020
Merged

Do not run any cleanup if the program is exiting anyway #2372

merged 1 commit into from
Jan 21, 2020

Conversation

martin-frbg
Copy link
Collaborator

From keno's PR #2350 - this avoids the potential hang in blas_thread_shutdown where we may wait for threads to exit while they are waiting on the loader lock from DllMain

From keno's PR #2350 - this avoids the potential hang in blas_thread_shutdown where we may wait for threads to exit while they are waiting on the loader lock from DllMain
@martin-frbg martin-frbg merged commit e011ad8 into OpenMathLib:develop Jan 21, 2020
@Keno
Copy link
Contributor

Keno commented Feb 28, 2020

We continued seeing hangs even after this patch and after some additional investigation, this patch turns out to be insufficient. We have another copy of DllMain here: https://github.com/xianyi/OpenBLAS/blob/develop/driver/others/memory.c#L1551 (as well as one more in that file), as well as a bunch of CRT tricks. In addition gotoblas_quit is declared as DESTRUCTOR when built with gcc on windows, so that'll run it yet again. Ideally all those instances in memory.c need to be removed. Also that file basically duplicates all functions in it. It would be nice to clean that up a bit.

@martin-frbg
Copy link
Collaborator Author

Yes memory.c is messy, it is basically two versions of the same file rolled into one - owing to the bad experience with the TLS patches. And I suspect the DllMain there will only be invoked in static builds.

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.

2 participants