[WTF] Drop initializeDate
Created attachment 316092 [details] Patch
Comment on attachment 316092 [details] Patch Clearing flags on attachment: 316092 Committed r219732: <http://trac.webkit.org/changeset/219732>
All reviewed patches have been landed. Closing bug.
Comment on attachment 316092 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=316092&action=review > Source/WTF/ChangeLog:8 > + minYear static variable will be initialized in an exclusive manner, which is ensured by C++ "static" semantics. Is that true? I thought we were compiling with no-threadsafe-statics.
(In reply to Alexey Proskuryakov from comment #4) > Comment on attachment 316092 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=316092&action=review > > > Source/WTF/ChangeLog:8 > > + minYear static variable will be initialized in an exclusive manner, which is ensured by C++ "static" semantics. > > Is that true? I thought we were compiling with no-threadsafe-statics. I think our NeverDestroyed implementation relies on threadsafe-statics. Otherwise, we'll have issues, no?
Comment on attachment 316092 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=316092&action=review >>> Source/WTF/ChangeLog:8 >>> + minYear static variable will be initialized in an exclusive manner, which is ensured by C++ "static" semantics. >> >> Is that true? I thought we were compiling with no-threadsafe-statics. > > I think our NeverDestroyed implementation relies on threadsafe-statics. Otherwise, we'll have issues, no? Right, if we compiled WebKit with no-threadsafe-statics, we should already encounter issues with NeverDestroyed. I remember that I grepped this a bit while ago, and I can't find any no-threadsafe-statics in WebKit build settings. Only libwebrtc uses it. But WebKit itself does not. I've grepped again now and I can't find it.
We do compile with no-threadsafe-statics. We only use initialization NeverDestroyed when thread safety is not needed.
Many of the uses of LazyNeverDestroyed that use std::call_once are for thread safety.
Maybe I am wrong. If we really stopped using -fno-threadsafe-statics, then I can do another round of cleanup and get rid of almost all uses of LazyNeverDestroyed!
WebKit is built with -fno-threadsafe-statics. Perhaps there is some configuration that enables that, but all that I checked do not. See e.g. https://build.webkit.org/builders/Apple%20Sierra%20Release%20%28Build%29/builds/3721/steps/compile-webkit/logs/stdio
Checking wtf/DateMath.cpp specifically, it is also compiled with -fno-threadsafe-statics.
So did we create a thread safety problem or not? Can equivalentYearForDST be called from multiple threads?
I think we did, and I think this patch probably needs to be rolled out.
(In reply to Darin Adler from comment #7) > We do compile with no-threadsafe-statics. > > We only use initialization NeverDestroyed when thread safety is not needed. I think we have a problem. I remember that Mark changed NeverDestroyed<String>'s ASCIILiteral parameter to StaticStringImpl because this is not safe to access NeverDestroyed<String>. It means that some of NeverDestroyed<String> can be accessed from multiple threads. If so, without threadsafe statics, it is dangerous. I think we will see 1. constructor is called twice, it may break structure during the other thread uses it. 2. we may return half-baked structure if the other thread is constructing it right now. (In reply to Alexey Proskuryakov from comment #10) > WebKit is built with -fno-threadsafe-statics. Perhaps there is some > configuration that enables that, but all that I checked do not. > > See e.g. > https://build.webkit.org/builders/Apple%20Sierra%20Release%20%28Build%29/ > builds/3721/steps/compile-webkit/logs/stdio Wow, that's a problem! I didn't know that this is enabled since any xcodeproje does not specify it. We should adopt this option in CMake build files as well to clarify our build configuration. Filed. https://bugs.webkit.org/show_bug.cgi?id=174747 (In reply to Darin Adler from comment #13) > I think we did, and I think this patch probably needs to be rolled out. Yes, we should roll out.
Re-opened since this is blocked by bug 174748
For example, I think JSC/runtime/JSGlobalObjectFunctions.cpp's Bitmaps are racy if threadsafe statics is not ensured.
To make our programming easier we can start compiling all of WebKit with thread-safe statics if we can do performance tests and prove that it does not come at an unacceptable performance cost. At the time we originally turned that flag on it was critical for performance. Until then if we accidentally write code with thread safety problems then we should rewrite to avoid those problems.