Fix issue 18063 - thread_attachThis returns dangling pointer #1986
Conversation
acehreli
requested review from
andralex,
Burgos,
MartinNowak,
wilzbach and
PetarKirov
as code owners
|
Thanks for your pull request, @acehreli! We are looking forward to reviewing it, and you should be hearing from a maintainer soon. Some tips to help speed things up:
Bear in mind that large or tricky changes may require multiple rounds of review and revision. Please see CONTRIBUTING.md for more information. Bugzilla references
|
| @@ -2239,8 +2244,14 @@ version( Windows ) | |||
| */ | |||
| extern (C) void thread_detachThis() nothrow @nogc | |||
| { | |||
| Thread.slock.lock_nothrow(); | |||
| scope(exit) Thread.slock.unlock_nothrow(); | |||
There was a problem hiding this comment.
Ideally, these changes should be in the separate commit.
There was a problem hiding this comment.
Agreed but I could not pass my test program without fixing these as well. Perhaps I should open two bugs and submit fixes for two.
There was a problem hiding this comment.
Why not just add extra commit in this PR? Make the first commit to add locks, and the second commit (which includes Fixes issue ... entry) with t.setThis(null) and doc change?
acehreli
force-pushed
the
thread-detach-stability
branch
from
7bea663
to
da8f5c8
Compare
acehreli
force-pushed
the
thread-detach-stability
branch
from
da8f5c8
to
ec8560b
Compare
| { | ||
| t.stopRequested = true; | ||
| while (!t.stopped) | ||
| Thread.sleep(1000.msecs); |
There was a problem hiding this comment.
I hadn't realized that this sleep was so long. Reducing it to 1.mecs causes a segfault. Working on it...
|
Closing for now as I'm not sure whether SIGSEGV during pthread_detach() inside code/thread.d is a druntime or a test code bug. |
Prevent returning dangling pointer as well as call all detach varieties after locking slock. (All other calls to Thread.remove() are done while holding slock.)