Bug 187009 – Use std::call_once() instead of dispatch_once() in SoftLinking.h

• Component: Web Template Framework
• Status: RESOLVED WONTFIX

[David Kilzer (:ddkilzer)]

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.

[David Kilzer (:ddkilzer)]

Created attachment 343522 [details]
Patch v1

[David Kilzer (:ddkilzer)]

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>

[David Kilzer (:ddkilzer)]

Created attachment 343525 [details]
Patch v2

[EWS Watchlist]

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

[EWS Watchlist]

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

[David Kilzer (:ddkilzer)]

Comment on attachment 343525 [details]
Patch v2

This code isn't even used on Windows.

[David Kilzer (:ddkilzer)]

(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.

[Anders Carlsson]

Why is this better than using dispatch_once, which works everywhere and inlines the first guard variable check?

[David Kilzer (:ddkilzer)]

(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?

[Anders Carlsson]

(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.

[David Kilzer (:ddkilzer)]

(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.