WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
141209
Workaround a thread library bug where thread destructors may not get called
https://bugs.webkit.org/show_bug.cgi?id=141209
Summary
Workaround a thread library bug where thread destructors may not get called
Mark Lam
Reported
2015-02-03 13:48:53 PST
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.
Attachments
the patch
(6.01 KB, patch)
2015-02-03 14:02 PST
,
Mark Lam
msaboff
: review-
Details
Formatted Diff
Diff
patch 2: no more race condition.
(8.80 KB, patch)
2015-02-03 15:36 PST
,
Mark Lam
msaboff
: review+
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Mark Lam
Comment 1
2015-02-03 13:51:37 PST
<
rdar://problem/17979784
>
Mark Lam
Comment 2
2015-02-03 14:02:04 PST
Created
attachment 245965
[details]
the patch
Alexey Proskuryakov
Comment 3
2015-02-03 14:17:42 PST
> there's a darwin bug
Is there a Radar tracking that?
Michael Saboff
Comment 4
2015-02-03 14:29:58 PST
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.
Mark Lam
Comment 5
2015-02-03 15:36:21 PST
Created
attachment 245977
[details]
patch 2: no more race condition.
Michael Saboff
Comment 6
2015-02-03 15:48:42 PST
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.
Mark Lam
Comment 7
2015-02-03 15:55:43 PST
(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.
Mark Lam
Comment 8
2015-02-03 16:01:04 PST
Thanks for the review. Landed in
r179576
: <
http://trac.webkit.org/r179576
>.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug