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+
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
Radar WebKit Bug Importer
Comment 8 2018-05-26 14:00:21 PDT
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.