From 59375a015a8a0cfdb5a330242241ab3c8df3774a Mon Sep 17 00:00:00 2001 From: "Torne (Richard Coles)" Date: Mon, 18 Jul 2016 18:34:02 +0100 Subject: [PATCH] Remove synchronized from static methods in WebView. ART uses the class object's embedded monitor to protect class initialization - the same monitor used by "synchronized" applied to static methods. This can cause deadlock in extremely rare cases of preemption during WebView code loading. Remove synchronized from all static methods in WebView, to avoid this deadlock. In most cases the synchronized is simply unnecessary in the Chromium-based WebView implementation; in CookieSyncManager we instead use a separately allocated Lock object to preserve thread safety. Bug: 27417671 Change-Id: I13049f23fc984b77a3f4c203a5c698858c64abfe --- api/current.txt | 6 ++--- api/system-current.txt | 6 ++--- api/test-current.txt | 6 ++--- core/java/android/webkit/CookieManager.java | 2 +- .../android/webkit/CookieSyncManager.java | 25 +++++++++++-------- core/java/android/webkit/WebView.java | 2 +- 6 files changed, 26 insertions(+), 21 deletions(-) diff --git a/api/current.txt b/api/current.txt index f37cf94adfefc..33c527e0ee0bd 100644 --- a/api/current.txt +++ b/api/current.txt @@ -45242,7 +45242,7 @@ package android.webkit { method public static boolean allowFileSchemeCookies(); method public abstract void flush(); method public abstract java.lang.String getCookie(java.lang.String); - method public static synchronized android.webkit.CookieManager getInstance(); + method public static android.webkit.CookieManager getInstance(); method public abstract boolean hasCookies(); method public abstract deprecated void removeAllCookie(); method public abstract void removeAllCookies(android.webkit.ValueCallback); @@ -45257,8 +45257,8 @@ package android.webkit { } public final deprecated class CookieSyncManager extends android.webkit.WebSyncManager { - method public static synchronized android.webkit.CookieSyncManager createInstance(android.content.Context); - method public static synchronized android.webkit.CookieSyncManager getInstance(); + method public static android.webkit.CookieSyncManager createInstance(android.content.Context); + method public static android.webkit.CookieSyncManager getInstance(); method protected deprecated void syncFromRamToFlash(); field protected static final java.lang.String LOGTAG = "websync"; field protected android.webkit.WebViewDatabase mDataBase; diff --git a/api/system-current.txt b/api/system-current.txt index 851a5b1142f1b..806a0e9d644b5 100644 --- a/api/system-current.txt +++ b/api/system-current.txt @@ -48414,7 +48414,7 @@ package android.webkit { method public abstract java.lang.String getCookie(java.lang.String); method public abstract java.lang.String getCookie(java.lang.String, boolean); method public synchronized java.lang.String getCookie(android.net.WebAddress); - method public static synchronized android.webkit.CookieManager getInstance(); + method public static android.webkit.CookieManager getInstance(); method public abstract boolean hasCookies(); method public abstract boolean hasCookies(boolean); method public abstract deprecated void removeAllCookie(); @@ -48431,8 +48431,8 @@ package android.webkit { } public final deprecated class CookieSyncManager extends android.webkit.WebSyncManager { - method public static synchronized android.webkit.CookieSyncManager createInstance(android.content.Context); - method public static synchronized android.webkit.CookieSyncManager getInstance(); + method public static android.webkit.CookieSyncManager createInstance(android.content.Context); + method public static android.webkit.CookieSyncManager getInstance(); method protected deprecated void syncFromRamToFlash(); field protected static final java.lang.String LOGTAG = "websync"; field protected android.webkit.WebViewDatabase mDataBase; diff --git a/api/test-current.txt b/api/test-current.txt index eeea51b2325eb..be910cfed3bc5 100644 --- a/api/test-current.txt +++ b/api/test-current.txt @@ -45322,7 +45322,7 @@ package android.webkit { method public static boolean allowFileSchemeCookies(); method public abstract void flush(); method public abstract java.lang.String getCookie(java.lang.String); - method public static synchronized android.webkit.CookieManager getInstance(); + method public static android.webkit.CookieManager getInstance(); method public abstract boolean hasCookies(); method public abstract deprecated void removeAllCookie(); method public abstract void removeAllCookies(android.webkit.ValueCallback); @@ -45337,8 +45337,8 @@ package android.webkit { } public final deprecated class CookieSyncManager extends android.webkit.WebSyncManager { - method public static synchronized android.webkit.CookieSyncManager createInstance(android.content.Context); - method public static synchronized android.webkit.CookieSyncManager getInstance(); + method public static android.webkit.CookieSyncManager createInstance(android.content.Context); + method public static android.webkit.CookieSyncManager getInstance(); method protected deprecated void syncFromRamToFlash(); field protected static final java.lang.String LOGTAG = "websync"; field protected android.webkit.WebViewDatabase mDataBase; diff --git a/core/java/android/webkit/CookieManager.java b/core/java/android/webkit/CookieManager.java index 3ec235b6e3cd6..67289c28e7a1e 100644 --- a/core/java/android/webkit/CookieManager.java +++ b/core/java/android/webkit/CookieManager.java @@ -35,7 +35,7 @@ public abstract class CookieManager { * * @return the singleton CookieManager instance */ - public static synchronized CookieManager getInstance() { + public static CookieManager getInstance() { return WebViewFactory.getProvider().getCookieManager(); } diff --git a/core/java/android/webkit/CookieSyncManager.java b/core/java/android/webkit/CookieSyncManager.java index eda8d3605112d..c974b328d70fb 100644 --- a/core/java/android/webkit/CookieSyncManager.java +++ b/core/java/android/webkit/CookieSyncManager.java @@ -65,6 +65,7 @@ public final class CookieSyncManager extends WebSyncManager { private static CookieSyncManager sRef; private static boolean sGetInstanceAllowed = false; + private static final Object sLock = new Object(); private CookieSyncManager() { super(null, null); @@ -77,12 +78,14 @@ public final class CookieSyncManager extends WebSyncManager { * * @return CookieSyncManager */ - public static synchronized CookieSyncManager getInstance() { - checkInstanceIsAllowed(); - if (sRef == null) { - sRef = new CookieSyncManager(); + public static CookieSyncManager getInstance() { + synchronized (sLock) { + checkInstanceIsAllowed(); + if (sRef == null) { + sRef = new CookieSyncManager(); + } + return sRef; } - return sRef; } /** @@ -90,12 +93,14 @@ public final class CookieSyncManager extends WebSyncManager { * @param context * @return CookieSyncManager */ - public static synchronized CookieSyncManager createInstance(Context context) { - if (context == null) { - throw new IllegalArgumentException("Invalid context argument"); + public static CookieSyncManager createInstance(Context context) { + synchronized (sLock) { + if (context == null) { + throw new IllegalArgumentException("Invalid context argument"); + } + setGetInstanceIsAllowed(); + return getInstance(); } - setGetInstanceIsAllowed(); - return getInstance(); } /** diff --git a/core/java/android/webkit/WebView.java b/core/java/android/webkit/WebView.java index cbfe43a230f6b..6f3fa3685b8d2 100644 --- a/core/java/android/webkit/WebView.java +++ b/core/java/android/webkit/WebView.java @@ -2322,7 +2322,7 @@ public class WebView extends AbsoluteLayout } } - private static synchronized WebViewFactoryProvider getFactory() { + private static WebViewFactoryProvider getFactory() { return WebViewFactory.getProvider(); }