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

What's the reason for TerminateThread? #3785

Closed
lb90 opened this issue Oct 11, 2022 · 4 comments
Closed

What's the reason for TerminateThread? #3785

lb90 opened this issue Oct 11, 2022 · 4 comments

Comments

@lb90
Copy link

lb90 commented Oct 11, 2022

Hi! Many thanks for the excellent work!

I see that OpenBLAS on Windows may end up calling TerminateThread. This is very dangerous API which should never be used under normal circumstances, as described in 1f60715

May I ask what's the reason for the TerminateThread call? I see that it's also excluded in UWP builds, could we just remove it?

Thank you very much!!!
Luca

@brada4
Copy link
Contributor

brada4 commented Oct 11, 2022

Linux equivalent is https://github.com/xianyi/OpenBLAS/blob/eece0dfd143013ca6572a8d3750af159209eb019/driver/others/blas_server.c#L1083

I think windows API does not have equivalent, one has to post CloseThread to the thread so it goes away, if it is busy at the point, i.e inside BLAS computation the terminatethread is the only option. i.e we shoot the cannon at the sparrow, and assume it went away, while we could try to whistle n clap first.

@martin-frbg
Copy link
Collaborator

From git blame, this appears to have been originally put in by xianyi (a294245) to "fix" a potential hang when freeing the dll (reported as #307).
There has been an effort to remove this code in #2350, but only a small part of that PR got merged as the key change was found to cause freezes in other code (namely GIMP which does a rapid load/unload cycle of all its dependencies on startup)

@brada4
Copy link
Contributor

brada4 commented Oct 12, 2022

I think it is a call for some softer albeit more complex thread exit

@lb90
Copy link
Author

lb90 commented Oct 12, 2022

Thanks for the informations! I guess I can work on that

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

No branches or pull requests

3 participants