Changeset 246652 in webkit

Timestamp:

Jun 20, 2019 1:39:29 PM (5 years ago)

Author:

Brent Fulgham

Message:

Resolve frequent crashes in topPrivatelyControlledDomain
​https://bugs.webkit.org/show_bug.cgi?id=199072
<rdar://problem/51428162>

Reviewed by Youenn Fablet.

Crash data indicates that we are frequently crashing when multiple threads call
WebCore::topPrivatelyControlledDomain.

Code review showed the potential for a thread contention issue, since WebKit builds
with '--fno-threadsafe-statics'.

This patch corrects the thread safety issue in WebCore::topPrivatelyControlledDomain.

• platform/mac/PublicSuffixMac.mm:

(WebCore::topPrivatelyControlledDomain): Only instantiate the static cache after the
current thread has achieved its lock.

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/mac/PublicSuffixMac.mm (modified) (1 diff)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-06-20  Brent Fulgham  <bfulgham@apple.com>
+
+        Resolve frequent crashes in topPrivatelyControlledDomain
+        https://bugs.webkit.org/show_bug.cgi?id=199072
+        <rdar://problem/51428162>
+
+        Reviewed by Youenn Fablet.
+
+        Crash data indicates that we are frequently crashing when multiple threads call
+        WebCore::topPrivatelyControlledDomain. 
+
+        Code review showed the potential for a thread contention issue, since WebKit builds
+        with '--fno-threadsafe-statics'.
+
+        This patch corrects the thread safety issue in WebCore::topPrivatelyControlledDomain.
+
+        * platform/mac/PublicSuffixMac.mm:
+        (WebCore::topPrivatelyControlledDomain): Only instantiate the static cache after the
+        current thread has achieved its lock.
+
 2019-06-20  Saam Barati  <sbarati@apple.com>
 

trunk/Source/WebCore/platform/mac/PublicSuffixMac.mm

@@ -51 +51 @@
         return domain;
 
+    static Lock cacheLock;
+    auto locker = holdLock(cacheLock);
+
     static NeverDestroyed<HashMap<String, String, ASCIICaseInsensitiveHash>> cache;
-    static Lock cacheLock;
 
     auto isolatedDomain = domain.isolatedCopy();
-
-    auto locker = holdLock(cacheLock);
-
+    
     constexpr auto maximumSizeToPreventUnlimitedGrowth = 128;
     if (cache.get().size() == maximumSizeToPreventUnlimitedGrowth)