WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
186014
testair sometimes crashes due to races in initialization of ARC4RandomNumberGenerator
https://bugs.webkit.org/show_bug.cgi?id=186014
Summary
testair sometimes crashes due to races in initialization of ARC4RandomNumberG...
Filip Pizlo
Reported
2018-05-26 12:32:52 PDT
Patch forthcoming.
Attachments
the patch
(5.00 KB, patch)
2018-05-26 12:35 PDT
,
Filip Pizlo
ysuzuki
: review+
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Filip Pizlo
Comment 1
2018-05-26 12:35:58 PDT
Created
attachment 341405
[details]
the patch
Yusuke Suzuki
Comment 2
2018-05-26 13:18:59 PDT
Comment on
attachment 341405
[details]
the patch View in context:
https://bugs.webkit.org/attachment.cgi?id=341405&action=review
r=me I think WTF::RandomDevice would reuire similar change.
> Source/JavaScriptCore/runtime/JSString.cpp:58 > + dataLog("Really destroying string.\n");
This part seems accidentally included.
> Source/WTF/wtf/CryptographicallyRandomNumber.cpp:162 > + static ARC4RandomNumberGenerator* randomNumberGenerator;
I think LazyNeverDestroyed is better.
Filip Pizlo
Comment 3
2018-05-26 13:53:11 PDT
(In reply to Yusuke Suzuki from
comment #2
)
> Comment on
attachment 341405
[details]
> the patch > > View in context: >
https://bugs.webkit.org/attachment.cgi?id=341405&action=review
> > r=me I think WTF::RandomDevice would reuire similar change.
Doesn't look like that uses NeverDestroyed<>.
> > > Source/JavaScriptCore/runtime/JSString.cpp:58 > > + dataLog("Really destroying string.\n"); > > This part seems accidentally included.
Ooops!
> > > Source/WTF/wtf/CryptographicallyRandomNumber.cpp:162 > > + static ARC4RandomNumberGenerator* randomNumberGenerator; > > I think LazyNeverDestroyed is better.
I'll try that.
Filip Pizlo
Comment 4
2018-05-26 13:55:33 PDT
(In reply to Filip Pizlo from
comment #3
)
> (In reply to Yusuke Suzuki from
comment #2
) > > Comment on
attachment 341405
[details]
> > the patch > > > > View in context: > >
https://bugs.webkit.org/attachment.cgi?id=341405&action=review
> > > > r=me I think WTF::RandomDevice would reuire similar change. > > Doesn't look like that uses NeverDestroyed<>. > > > > > > Source/JavaScriptCore/runtime/JSString.cpp:58 > > > + dataLog("Really destroying string.\n"); > > > > This part seems accidentally included. > > Ooops! > > > > > > Source/WTF/wtf/CryptographicallyRandomNumber.cpp:162 > > > + static ARC4RandomNumberGenerator* randomNumberGenerator; > > > > I think LazyNeverDestroyed is better. > > I'll try that.
LazyNeverDestroyed has the same issue.
Filip Pizlo
Comment 5
2018-05-26 13:56:07 PDT
(In reply to Filip Pizlo from
comment #4
)
> (In reply to Filip Pizlo from
comment #3
) > > (In reply to Yusuke Suzuki from
comment #2
) > > > Comment on
attachment 341405
[details]
> > > the patch > > > > > > View in context: > > >
https://bugs.webkit.org/attachment.cgi?id=341405&action=review
> > > > > > r=me I think WTF::RandomDevice would reuire similar change. > > > > Doesn't look like that uses NeverDestroyed<>. > > > > > > > > > Source/JavaScriptCore/runtime/JSString.cpp:58 > > > > + dataLog("Really destroying string.\n"); > > > > > > This part seems accidentally included. > > > > Ooops! > > > > > > > > > Source/WTF/wtf/CryptographicallyRandomNumber.cpp:162 > > > > + static ARC4RandomNumberGenerator* randomNumberGenerator; > > > > > > I think LazyNeverDestroyed is better. > > > > I'll try that. > > LazyNeverDestroyed has the same issue.
To clarify: it has the same race as NeverDestroyed. So, using that would not fix the bug.
Filip Pizlo
Comment 6
2018-05-26 13:57:19 PDT
(In reply to Filip Pizlo from
comment #5
)
> (In reply to Filip Pizlo from
comment #4
) > > (In reply to Filip Pizlo from
comment #3
) > > > (In reply to Yusuke Suzuki from
comment #2
) > > > > Comment on
attachment 341405
[details]
> > > > the patch > > > > > > > > View in context: > > > >
https://bugs.webkit.org/attachment.cgi?id=341405&action=review
> > > > > > > > r=me I think WTF::RandomDevice would reuire similar change. > > > > > > Doesn't look like that uses NeverDestroyed<>. > > > > > > > > > > > > Source/JavaScriptCore/runtime/JSString.cpp:58 > > > > > + dataLog("Really destroying string.\n"); > > > > > > > > This part seems accidentally included. > > > > > > Ooops! > > > > > > > > > > > > Source/WTF/wtf/CryptographicallyRandomNumber.cpp:162 > > > > > + static ARC4RandomNumberGenerator* randomNumberGenerator; > > > > > > > > I think LazyNeverDestroyed is better. > > > > > > I'll try that. > > > > LazyNeverDestroyed has the same issue. > > To clarify: it has the same race as NeverDestroyed. So, using that would > not fix the bug.
Wait, do you mean using LazyNeverDestroyed instead of pointing to it? Took me a while to figure that out.
Filip Pizlo
Comment 7
2018-05-26 13:59:30 PDT
Landed in
https://trac.webkit.org/changeset/232227/webkit
Radar WebKit Bug Importer
Comment 8
2018-05-26 14:00:21 PDT
<
rdar://problem/40581599
>
Yusuke Suzuki
Comment 9
2018-05-27 06:52:59 PDT
(In reply to Filip Pizlo from
comment #6
)
> (In reply to Filip Pizlo from
comment #5
) > > (In reply to Filip Pizlo from
comment #4
) > > > (In reply to Filip Pizlo from
comment #3
) > > > > (In reply to Yusuke Suzuki from
comment #2
) > > > > > Comment on
attachment 341405
[details]
> > > > > the patch > > > > > > > > > > View in context: > > > > >
https://bugs.webkit.org/attachment.cgi?id=341405&action=review
> > > > > > > > > > r=me I think WTF::RandomDevice would reuire similar change. > > > > > > > > Doesn't look like that uses NeverDestroyed<>. > > > > > > > > > > > > > > > Source/JavaScriptCore/runtime/JSString.cpp:58 > > > > > > + dataLog("Really destroying string.\n"); > > > > > > > > > > This part seems accidentally included. > > > > > > > > Ooops! > > > > > > > > > > > > > > > Source/WTF/wtf/CryptographicallyRandomNumber.cpp:162 > > > > > > + static ARC4RandomNumberGenerator* randomNumberGenerator; > > > > > > > > > > I think LazyNeverDestroyed is better. > > > > > > > > I'll try that. > > > > > > LazyNeverDestroyed has the same issue. > > > > To clarify: it has the same race as NeverDestroyed. So, using that would > > not fix the bug. > > Wait, do you mean using LazyNeverDestroyed instead of pointing to it? Took > me a while to figure that out.
Yeah, I mean using LazyNeverDestroyed<> instead of pointer (and calling construct() in call_once). :)
Yusuke Suzuki
Comment 10
2018-05-27 06:54:25 PDT
(In reply to Filip Pizlo from
comment #3
)
> (In reply to Yusuke Suzuki from
comment #2
) > > Comment on
attachment 341405
[details]
> > the patch > > > > View in context: > >
https://bugs.webkit.org/attachment.cgi?id=341405&action=review
> > > > r=me I think WTF::RandomDevice would reuire similar change. > > Doesn't look like that uses NeverDestroyed<>. > > > > > > Source/JavaScriptCore/runtime/JSString.cpp:58 > > > + dataLog("Really destroying string.\n"); > > > > This part seems accidentally included. > > Ooops! > > > > > > Source/WTF/wtf/CryptographicallyRandomNumber.cpp:162 > > > + static ARC4RandomNumberGenerator* randomNumberGenerator; > > > > I think LazyNeverDestroyed is better. > > I'll try that.
RandomDevice one is OSRandomSource.cpp's NeverDestroyed<RandomDevice>. I think it has similar problem.
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