Issue 6366 - Elemhide thread is accessing released engine

libadblockplus-android-webview/src/org/adblockplus/libadblockplus/android/webview/AdblockWebView.java

 /*
  * This file is part of Adblock Plus <https://adblockplus.org/>,
  * Copyright (C) 2006-present eyeo GmbH
  *
  * Adblock Plus is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License version 3 as
  * published by the Free Software Foundation.
  *
  * Adblock Plus is distributed in the hope that it will be useful,
  * but WITHOUT ANY WARRANTY; without even the implied warranty of
@@ skipping 800 matching lines @@
       else
       {
         super.onReceivedLoginRequest(view, realm, account, args);
       }
     }
 
     protected WebResourceResponse shouldInterceptRequest(
       WebView webview, String url, boolean isMainFrame,
       boolean isXmlHttpRequest, String[] referrerChainArray)
     {
-      // if dispose() was invoke, but the page is still loading then just let it go
-      if (provider.getCounter() == 0)
+      synchronized (provider.getEngineLock())
       {
-        e("FilterEngine already disposed, allow loading");
+        // if dispose() was invoke, but the page is still loading then just let it go

[diegocarloslima, 2018/02/07 14:00:06]

Not really related to the issue, but it seems that it should be 'invoked' here

[anton, 2018/02/07 14:03:22]

Yes, it should be `invoked`.
Not fixing now as it's not related change.

+        if (provider.getCounter() == 0)
+        {
+          e("FilterEngine already disposed, allow loading");
 
-        // allow loading by returning null
-        return null;
-      }
-      else
-      {
-        provider.waitForReady();
-      }
-
-      if (isMainFrame)
-      {
-        // never blocking main frame requests, just subrequests
-        w(url + " is main frame, allow loading");
-
-        // allow loading by returning null
-        return null;
-      }
-
-      // whitelisted
-      if (provider.getEngine().isDomainWhitelisted(url, referrerChainArray))
-      {
-        w(url + " domain is whitelisted, allow loading");
-
-        // allow loading by returning null
-        return null;
-      }
-
-      if (provider.getEngine().isDocumentWhitelisted(url, referrerChainArray))
-      {
-        w(url + " document is whitelisted, allow loading");
-
-        // allow loading by returning null
-        return null;
-      }
-
-      // determine the content
-      FilterEngine.ContentType contentType;
-      if (isXmlHttpRequest)
-      {
-        contentType = FilterEngine.ContentType.XMLHTTPREQUEST;
-      }
-      else
-      {
-        if (RE_JS.matcher(url).find())
-        {
-          contentType = FilterEngine.ContentType.SCRIPT;
-        }
-        else if (RE_CSS.matcher(url).find())
-        {
-          contentType = FilterEngine.ContentType.STYLESHEET;
-        }
-        else if (RE_IMAGE.matcher(url).find())
-        {
-          contentType = FilterEngine.ContentType.IMAGE;
-        }
-        else if (RE_FONT.matcher(url).find())
-        {
-          contentType = FilterEngine.ContentType.FONT;
-        }
-        else if (RE_HTML.matcher(url).find())
-        {
-          contentType = FilterEngine.ContentType.SUBDOCUMENT;
+          // allow loading by returning null
+          return null;
         }
         else
         {
-          contentType = FilterEngine.ContentType.OTHER;
+          provider.waitForReady();
         }
+
+        if (isMainFrame)
+        {
+          // never blocking main frame requests, just subrequests
+          w(url + " is main frame, allow loading");
+
+          // allow loading by returning null
+          return null;
+        }
+
+        // whitelisted
+        if (provider.getEngine().isDomainWhitelisted(url, referrerChainArray))
+        {
+          w(url + " domain is whitelisted, allow loading");
+
+          // allow loading by returning null
+          return null;
+        }
+
+        if (provider.getEngine().isDocumentWhitelisted(url, referrerChainArray))
+        {
+          w(url + " document is whitelisted, allow loading");
+
+          // allow loading by returning null
+          return null;
+        }
+
+        // determine the content
+        FilterEngine.ContentType contentType;
+        if (isXmlHttpRequest)
+        {
+          contentType = FilterEngine.ContentType.XMLHTTPREQUEST;
+        }
+        else
+        {
+          if (RE_JS.matcher(url).find())
+          {
+            contentType = FilterEngine.ContentType.SCRIPT;
+          }
+          else if (RE_CSS.matcher(url).find())
+          {
+            contentType = FilterEngine.ContentType.STYLESHEET;
+          }
+          else if (RE_IMAGE.matcher(url).find())
+          {
+            contentType = FilterEngine.ContentType.IMAGE;
+          }
+          else if (RE_FONT.matcher(url).find())
+          {
+            contentType = FilterEngine.ContentType.FONT;
+          }
+          else if (RE_HTML.matcher(url).find())
+          {
+            contentType = FilterEngine.ContentType.SUBDOCUMENT;
+          }
+          else
+          {
+            contentType = FilterEngine.ContentType.OTHER;
+          }
+        }
+
+        // check if we should block
+        if (provider.getEngine().matches(url, contentType, referrerChainArray))
+        {
+          w("Blocked loading " + url);
+
+          // if we should block, return empty response which results in 'errorLoading' callback
+          return new WebResourceResponse("text/plain", "UTF-8", null);
+        }
+
+        d("Allowed loading " + url);
+
+        // continue by returning null
+        return null;
       }
-
-      // check if we should block
-      if (provider.getEngine().matches(url, contentType, referrerChainArray))
-      {
-        w("Blocked loading " + url);
-
-        // if we should block, return empty response which results in 'errorLoading' callback
-        return new WebResourceResponse("text/plain", "UTF-8", null);
-      }
-
-      d("Allowed loading " + url);
-
-      // continue by returning null
-      return null;
     }
 
     @TargetApi(Build.VERSION_CODES.LOLLIPOP)
     @Override
     public WebResourceResponse shouldInterceptRequest(WebView view, WebResourceRequest request)
     {
       // here we just trying to fill url -> referrer map
       // blocking/allowing loading will happen in `shouldInterceptRequest(WebView,String)`
       String url = request.getUrl().toString();
 
@@ skipping 47 matching lines @@
     public ElemHideThread(CountDownLatch finishedLatch)
     {
       this.finishedLatch = finishedLatch;
       isFinished = new AtomicBoolean(false);
       isCancelled = new AtomicBoolean(false);
     }
 
     @Override
     public void run()
     {
-      try
+      synchronized (provider.getEngineLock())
       {
-        if (provider.getCounter() == 0)
+        try
         {
-          w("FilterEngine already disposed");
-          selectorsString = EMPTY_ELEMHIDE_ARRAY_STRING;
-        }
-        else
-        {
-          provider.waitForReady();
-          String[] referrers = new String[]
-            {
-              url
-            };
-
-          List<Subscription> subscriptions = provider
-            .getEngine()
-            .getFilterEngine()
-            .getListedSubscriptions();
-
-          try
+          if (provider.getCounter() == 0)
           {
-            d("Listed subscriptions: " + subscriptions.size());
-            if (debugMode)
-            {
-              for (Subscription eachSubscription : subscriptions)
-              {
-                d("Subscribed to "
-                  + (eachSubscription.isDisabled() ? "disabled" : "enabled")
-                  + " " + eachSubscription);
-              }
-            }
-          }
-          finally
-          {
-            for (Subscription eachSubscription : subscriptions)
-            {
-              eachSubscription.dispose();
-            }
-          }
-
-          final String domain = provider.getEngine().getFilterEngine().getHostFromURL(url);
-          if (domain == null)
-          {
-            e("Failed to extract domain from " + url);
+            w("FilterEngine already disposed");
             selectorsString = EMPTY_ELEMHIDE_ARRAY_STRING;
           }
           else
           {
-            d("Requesting elemhide selectors from AdblockEngine for " + url + " in " + this);
-            List<String> selectors = provider
+            provider.waitForReady();
+            String[] referrers = new String[]
+              {
+                url
+              };
+
+            List<Subscription> subscriptions = provider
               .getEngine()
-              .getElementHidingSelectors(url, domain, referrers);
+              .getFilterEngine()
+              .getListedSubscriptions();
 
-            d("Finished requesting elemhide selectors, got " + selectors.size() + " in " + this);
-            selectorsString = Utils.stringListToJsonArray(selectors);
+            try
+            {
+              d("Listed subscriptions: " + subscriptions.size());
+              if (debugMode)
+              {
+                for (Subscription eachSubscription : subscriptions)
+                {
+                  d("Subscribed to "
+                    + (eachSubscription.isDisabled() ? "disabled" : "enabled")
+                    + " " + eachSubscription);
+                }
+              }
+            }
+            finally
+            {
+              for (Subscription eachSubscription : subscriptions)
+              {
+                eachSubscription.dispose();
+              }
+            }
+
+            final String domain = provider.getEngine().getFilterEngine().getHostFromURL(url);
+            if (domain == null)
+            {
+              e("Failed to extract domain from " + url);
+              selectorsString = EMPTY_ELEMHIDE_ARRAY_STRING;
+            }
+            else
+            {
+              d("Requesting elemhide selectors from AdblockEngine for " + url + " in " + this);
+              List<String> selectors = provider
+                .getEngine()
+                .getElementHidingSelectors(url, domain, referrers);
+
+              d("Finished requesting elemhide selectors, got " + selectors.size() + " in " + this);
+              selectorsString = Utils.stringListToJsonArray(selectors);
+            }
           }
         }
-      }
-      finally
-      {
-        if (isCancelled.get())
+        finally
         {
-          w("This thread is cancelled, exiting silently " + this);
-        }
-        else
-        {
-          finish(selectorsString);
+          if (isCancelled.get())
+          {
+            w("This thread is cancelled, exiting silently " + this);
+          }
+          else
+          {
+            finish(selectorsString);
+          }
         }
       }
     }
 
     private void onFinished()
     {
       finishedLatch.countDown();
       synchronized (finishedRunnableLockObject)
       {
         if (finishedRunnable != null)
@@ skipping 380 matching lines @@
         w("Busy with elemhide selectors, delayed disposing scheduled");
         elemHideThread.setFinishedRunnable(disposeRunnable);
       }
       else
       {
         disposeRunnable.run();
       }
     }
   }
 }