Really excise tinythread #4106
Really excise tinythread #4106
Conversation
|
Looks like it's a case of double locking the mutex in PluginManager's Plugin::RefLock wait() line 106. So yeah, should be able to reproduce. |
library/PlugLoad-windows.cpp
Outdated
| @@ -34,7 +34,7 @@ distribution. | |||
| #include "Hooks.h" | |||
| #include <stdio.h> | |||
|
|
|||
| #include "tinythread.h" | |||
| #include "../plugins/uicommon.h" | |||
There was a problem hiding this comment.
why is this included? it concerns me that a plugin header is included from Core. Moreover, what is Core doing that is provided by uicommon?
There was a problem hiding this comment.
I'll have to look into that. Wasn't me that put that in there, at least on purpose. Could've been a rebase screwup.
|
tests show what appears to be a deadlock on Windows. looking at the test artifacts, it appears that the hang happens as soon as the |
|
That sucks. It seems to run ok on Linux, including the tests. |
dhthwy
force-pushed
the
really-excise-tinythread
branch
2 times, most recently
from
cf45476 to
df38aaa
Compare
Remove useless tinythread import in PlugLoad-windows.cpp Remove seemingly useless tinythread import in LuaTools.cpp Factor out tinythread in LuaApi.cpp Removed unused tinythread in LuaWrapper.cpp Removed unused tinythread include in LuaTypes.cpp Removed unused tinythread include in ColorText.cpp Factor out tinythread in Console.h Factor out tinythread in Console-posix.cpp Factor out tinythread in Console-windows.cpp Factor out tinythread in renderer_light Factor out tinythread in DataDefs.cpp Remove unused tinythread include in RemoteClient.cpp Add includes for new mutex and conditional_variable usages in PluginManager Factor out tinythread from devel/memview, renderermax/renderer_light, and rendermax/renderer_opengl plugins Remove usages of tinythread in various CMakeLists.txt files, in .ycm_extra_conf.py, and delete tinythread itself Delete tinythread from LISCENSE.rst excise tinythread: fix deadlock in pluginmanager excise tinythread: remove improper header excise tinythread: fix double unlock. fix plugin typo
dhthwy
force-pushed
the
really-excise-tinythread
branch
from
df38aaa to
54769eb
Compare
|
FWIW, I'd be happy if you just added a comment to rendermax that improvements could be made and that the threading needs verification and leave it at that. I wouldn't want it holding this PR up indefinitely. |
|
Whichever is fine. A comment is a good start at any rate. |
|
I did change the isDone bool to an std::atomic. I'm confident (not that it means much) that change won't do any harm except for make it run a tiny bit slower. But that said, I can revert that commit if you'd rather keep it separate and worry about it later. |
rendermax: note that this plugin needs testing and improvements
dhthwy
force-pushed
the
really-excise-tinythread
branch
from
174bc8e to
ee61835
Compare
Takeover #2516
There is a deadlock when quitting df. I haven't looked into it much yet.@ab9rf will you test this on Windows when you get a chance?
And I'm curious if the hang is reproducible there.