As far as we know, there's a darwin bug where thread destructors may not get called. We will work around this by checking at GC time if the platform thread is still valid. If not, we'll purge it from the VM's registeredThreads list before proceeding with thread scanning activity.
<rdar://problem/17979784>
Created attachment 245965 [details] the patch
> there's a darwin bug Is there a Radar tracking that?
Comment on attachment 245965 [details] the patch r- I believe that there is a race condition. Consider thread A is exiting and it has already called MachineThreads::removeThread() but hasn't really exited. Then thread B executes this code and doesn't purge thread A. After completing this code, thread A really exits and then we call tryCopyOtherThreadStacks(). We'll still try to get thread stack info from the now dead A.
Created attachment 245977 [details] patch 2: no more race condition.
Comment on attachment 245977 [details] patch 2: no more race condition. View in context: https://bugs.webkit.org/attachment.cgi?id=245977&action=review r=me with a couple of minor changes. > Source/JavaScriptCore/ChangeLog:15 > + suspension, because the thread is only guaranteed to be alive while it Might want to change "the thread" to "another thread". > Source/JavaScriptCore/heap/MachineStackMarker.cpp:194 > +void MachineThreads::removeThreadWithLockAlreadyAcquired(PlatformThread platformThread) threadToRemove seems like a better name for the argument.
(In reply to comment #6) > Comment on attachment 245977 [details] > patch 2: no more race condition. > > View in context: > https://bugs.webkit.org/attachment.cgi?id=245977&action=review > > r=me with a couple of minor changes. > > > Source/JavaScriptCore/ChangeLog:15 > > + suspension, because the thread is only guaranteed to be alive while it > > Might want to change "the thread" to "another thread". Will change the wording to me more clear that it the another thread whose validity we're checking. > > Source/JavaScriptCore/heap/MachineStackMarker.cpp:194 > > +void MachineThreads::removeThreadWithLockAlreadyAcquired(PlatformThread platformThread) > > threadToRemove seems like a better name for the argument. Changed.
Thanks for the review. Landed in r179576: <http://trac.webkit.org/r179576>.