Use std::call_once() instead of dispatch_once() in SoftLinking.h. It's a lower-level API so there's less that can go wrong. It does only support C++ source files, though, although that shouldn't be a problem for WebKit source.
Created attachment 343522 [details] Patch v1
I'm not sure, but this might fix some of the tests failing on the iOS 11 Simulator bots, which are crashing in soft-linking code for QuickLook: <https://build.webkit.org/builders/Apple%20iOS%2011%20Simulator%20Release%20WK2%20(Tests)?numbuilds=50> <https://build.webkit.org/builders/Apple%20iOS%2011%20Simulator%20Debug%20WK2%20(Tests)?numbuilds=50>
Created attachment 343525 [details] Patch v2
Comment on attachment 343525 [details] Patch v2 Attachment 343525 [details] did not pass win-ews (win): Output: https://webkit-queues.webkit.org/results/8343010 New failing tests: http/tests/security/contentSecurityPolicy/video-with-http-url-allowed-by-csp-media-src-star.html
Created attachment 343589 [details] Archive of layout-test-results from ews204 for win-future The attached test failures were seen while running run-webkit-tests on the win-ews. Bot: ews204 Port: win-future Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit
Comment on attachment 343525 [details] Patch v2 This code isn't even used on Windows.
(In reply to David Kilzer (:ddkilzer) from comment #2) > I'm not sure, but this might fix some of the tests failing on the iOS 11 > Simulator bots, which are crashing in soft-linking code for QuickLook: > > <https://build.webkit.org/builders/ > Apple%20iOS%2011%20Simulator%20Release%20WK2%20(Tests)?numbuilds=50> > <https://build.webkit.org/builders/ > Apple%20iOS%2011%20Simulator%20Debug%20WK2%20(Tests)?numbuilds=50> Nope. That regression was caused by r233120 from Bug 186924.
Why is this better than using dispatch_once, which works everywhere and inlines the first guard variable check?
(In reply to Anders Carlsson from comment #8) > Why is this better than using dispatch_once, which works everywhere and > inlines the first guard variable check? libdispatch is not standard outside of Apple platforms, although that argument is less compelling since SoftLinking.h is platform-specific. Also libdispatch.dylib links to a lot more libraries than libc++.dylib (via `otool -L`), so without knowing exactly when soft-linking is invoked, it seems like keeping the soft linking code "simpler" (using a mechanism with fewer dependencies) would be generally preferable. And to that point, it appears that dispatch_once() is re-entering itself in the layout test crashes that caused the patch for Bug 186924 to be rolled out: <https://bugs.webkit.org/show_bug.cgi?id=186924#c13> Results: <https://build.webkit.org/results/Apple%20iOS%2011%20Simulator%20Release%20WK2%20(Tests)/r233120%20(5744)/results.html> Crash: <https://build.webkit.org/results/Apple%20iOS%2011%20Simulator%20Release%20WK2%20(Tests)/r233120%20(5744)/css2.1/20110323/replaced-intrinsic-004-crash-log.txt> Any ideas what might be causing libdispatch.dylib to re-enter itself in that case?
(In reply to David Kilzer (:ddkilzer) from comment #9) > (In reply to Anders Carlsson from comment #8) > > Why is this better than using dispatch_once, which works everywhere and > > inlines the first guard variable check? > > libdispatch is not standard outside of Apple platforms, although that > argument is less compelling since SoftLinking.h is platform-specific. > Yeah, that's not a convincing argument :) > Also libdispatch.dylib links to a lot more libraries than libc++.dylib (via > `otool -L`), so without knowing exactly when soft-linking is invoked, it > seems like keeping the soft linking code "simpler" (using a mechanism with > fewer dependencies) would be generally preferable. This shouldn't matter since libdispatch.dylib is used by every other app on the system so even if it wasn't in the shared cache (it is), it should already be paged in. > > And to that point, it appears that dispatch_once() is re-entering itself in > the layout test crashes that caused the patch for Bug 186924 to be rolled > out: > > <https://bugs.webkit.org/show_bug.cgi?id=186924#c13> > Results: > <https://build.webkit.org/results/ > Apple%20iOS%2011%20Simulator%20Release%20WK2%20(Tests)/r233120%20(5744)/ > results.html> > Crash: > <https://build.webkit.org/results/ > Apple%20iOS%2011%20Simulator%20Release%20WK2%20(Tests)/r233120%20(5744)/css2. > 1/20110323/replaced-intrinsic-004-crash-log.txt> > > Any ideas what might be causing libdispatch.dylib to re-enter itself in that > case? That's odd, but I think switching to std::call_once papers over the issue.
(In reply to Anders Carlsson from comment #10) > (In reply to David Kilzer (:ddkilzer) from comment #9) > > (In reply to Anders Carlsson from comment #8) > > > Why is this better than using dispatch_once, which works everywhere and > > > inlines the first guard variable check? > > > > Also libdispatch.dylib links to a lot more libraries than libc++.dylib (via > > `otool -L`), so without knowing exactly when soft-linking is invoked, it > > seems like keeping the soft linking code "simpler" (using a mechanism with > > fewer dependencies) would be generally preferable. > > This shouldn't matter since libdispatch.dylib is used by every other app on > the system so even if it wasn't in the shared cache (it is), it should > already be paged in. Sorry, my comment wasn't about being paged in to memory as much as it was about the complexity of the implementation of various parts of libdispatch and all the weird corner cases that can be hit if you don't use the API correctly. However, I don't know of cases where dispatch_once() has such issues now, though. > > And to that point, it appears that dispatch_once() is re-entering itself in > > the layout test crashes that caused the patch for Bug 186924 to be rolled > > out: > > > > <https://bugs.webkit.org/show_bug.cgi?id=186924#c13> > > Results: > > <https://build.webkit.org/results/ > > Apple%20iOS%2011%20Simulator%20Release%20WK2%20(Tests)/r233120%20(5744)/ > > results.html> > > Crash: > > <https://build.webkit.org/results/ > > Apple%20iOS%2011%20Simulator%20Release%20WK2%20(Tests)/r233120%20(5744)/css2. > > 1/20110323/replaced-intrinsic-004-crash-log.txt> > > > > Any ideas what might be causing libdispatch.dylib to re-enter itself in that > > case? > > That's odd, but I think switching to std::call_once papers over the issue. Okay, marking as RESOLVED/WONTFIX.