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
Report deadlocked thread names in failures to stop emulation #9865
Conversation
{ | ||
atomic_wait_engine::raw_notify(0, get_native_id()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I took the liberaty to remove this hack as I think all thread code paths are CELL_OK now.
Utilities/Thread.cpp
Outdated
@@ -2336,9 +2336,8 @@ bool thread_base::join(bool dtor) const | |||
} | |||
|
|||
// Hacked for too sleepy threads (1ms) TODO: make sure it's unneeded and remove | |||
const auto timeout = dtor && Emu.IsStopped() ? atomic_wait_timeout{1'000'000} : atomic_wait_timeout::inf; | |||
const auto timeout = dtor && !thread_ctrl::get_current() ? atomic_wait_timeout{1'000'000} : atomic_wait_timeout::inf; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you just keep this line the same? It does't stand in the way of the declared purpose of the PR. On the other hand, checking get_current() is a very weird one.
PR name confuses me as hell. Their names were always reported as I remember. |
Not when watchdog times out such as in https://cdn.discordapp.com/attachments/277227681836302338/815638772644446269/unknown.png |
Utilities/Thread.cpp
Outdated
@@ -2336,9 +2336,8 @@ bool thread_base::join(bool dtor) const | |||
} | |||
|
|||
// Hacked for too sleepy threads (1ms) TODO: make sure it's unneeded and remove | |||
const auto timeout = dtor && Emu.IsStopped() ? atomic_wait_timeout{1'000'000} : atomic_wait_timeout::inf; | |||
const auto timeout = dtor && !thread_ctrl::get_current() ? atomic_wait_timeout{1'000'000} : atomic_wait_timeout::inf; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you just keep this line the same? It does't stand in the way of the declared purpose of the PR. On the other hand, checking get_current() is a very weird one.
Finally report which threads were failing to terminate at Emu.Stop().