Issue 6366 - Elemhide thread is accessing released engine

libadblockplus-android/src/org/adblockplus/libadblockplus/android/SingleInstanceEngineProvider.java

Index: libadblockplus-android/src/org/adblockplus/libadblockplus/android/SingleInstanceEngineProvider.java
diff --git a/libadblockplus-android/src/org/adblockplus/libadblockplus/android/SingleInstanceEngineProvider.java b/libadblockplus-android/src/org/adblockplus/libadblockplus/android/SingleInstanceEngineProvider.java
index 4706684fa5327562adc1e54eed3cc42e742fbc5f..7a69ddab437e6861fd8c56b0ed91addf148cc93e 100644
--- a/libadblockplus-android/src/org/adblockplus/libadblockplus/android/SingleInstanceEngineProvider.java
+++ b/libadblockplus-android/src/org/adblockplus/libadblockplus/android/SingleInstanceEngineProvider.java
@@ -58,6 +58,7 @@ public class SingleInstanceEngineProvider implements AdblockEngineProvider
     new LinkedList<EngineCreatedListener>();
   private List<EngineDisposedListener> engineDisposedListeners =
     new LinkedList<EngineDisposedListener>();
+  private Object engineLock = new Object();

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

This should be final

[jens, 2018/02/07 14:01:23]

Could be final

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

Acknowledged.

See patch set #3

 
   /*
     Simple ARC management for AdblockEngine
@@ -137,45 +138,48 @@ public class SingleInstanceEngineProvider implements AdblockEngineProvider
 
   private void createAdblock()
   {
-    ConnectivityManager connectivityManager =
-      (ConnectivityManager) context.getSystemService(Context.CONNECTIVITY_SERVICE);
-    IsAllowedConnectionCallback isAllowedConnectionCallback =
-      new IsAllowedConnectionCallbackImpl(connectivityManager);
-
-    Log.d(TAG, "Creating adblock engine ...");
-
-    AdblockEngine.Builder builder = AdblockEngine
-      .builder(
-        AdblockEngine.generateAppInfo(context, developmentBuild),
-        basePath)
-      .setIsAllowedConnectionCallback(isAllowedConnectionCallback)
-      .enableElementHiding(true);
-
-    if (v8IsolateProviderPtr != null)
+    Log.w(TAG, "Waiting for lock");
+    synchronized (engineLock)
     {
-      builder.useV8IsolateProvider(v8IsolateProviderPtr);
-    }
+      Log.d(TAG, "Creating adblock engine ...");
+      ConnectivityManager connectivityManager =
+        (ConnectivityManager) context.getSystemService(Context.CONNECTIVITY_SERVICE);
+      IsAllowedConnectionCallback isAllowedConnectionCallback =
+        new IsAllowedConnectionCallbackImpl(connectivityManager);
+
+      AdblockEngine.Builder builder = AdblockEngine
+        .builder(
+          AdblockEngine.generateAppInfo(context, developmentBuild),
+          basePath)
+        .setIsAllowedConnectionCallback(isAllowedConnectionCallback)
+        .enableElementHiding(true);
+
+      if (v8IsolateProviderPtr != null)
+      {
+        builder.useV8IsolateProvider(v8IsolateProviderPtr);
+      }
 
-    // if preloaded subscriptions provided
-    if (preloadedPreferenceName != null)
-    {
-      SharedPreferences preloadedSubscriptionsPrefs = context.getSharedPreferences(
-        preloadedPreferenceName,
-        Context.MODE_PRIVATE);
-      builder.preloadSubscriptions(
-        context,
-        urlToResourceIdMap,
-        new AndroidWebRequestResourceWrapper.SharedPrefsStorage(preloadedSubscriptionsPrefs));
-    }
+      // if preloaded subscriptions provided
+      if (preloadedPreferenceName != null)
+      {
+        SharedPreferences preloadedSubscriptionsPrefs = context.getSharedPreferences(
+          preloadedPreferenceName,
+          Context.MODE_PRIVATE);
+        builder.preloadSubscriptions(
+          context,
+          urlToResourceIdMap,
+          new AndroidWebRequestResourceWrapper.SharedPrefsStorage(preloadedSubscriptionsPrefs));
+      }
 
-    engine = builder.build();
+      engine = builder.build();
 
-    Log.d(TAG, "AdblockHelper engine created");
+      Log.d(TAG, "AdblockHelper engine created");
 
-    // sometimes we need to init AdblockEngine instance, eg. set user settings
-    for (EngineCreatedListener listener : engineCreatedListeners)
-    {
-      listener.onAdblockEngineCreated(engine);
+      // sometimes we need to init AdblockEngine instance, eg. set user settings
+      for (EngineCreatedListener listener : engineCreatedListeners)
+      {
+        listener.onAdblockEngineCreated(engine);
+      }
     }
   }
 
@@ -202,10 +206,13 @@ public class SingleInstanceEngineProvider implements AdblockEngineProvider
           @Override
           public void run()
           {
-            createAdblock();
+            synchronized (engineLock)
+            {
+              createAdblock();
 
-            // unlock waiting client thread
-            engineCreated.countDown();
+              // unlock waiting client thread
+              engineCreated.countDown();
+            }
           }
         }).start();
       }
@@ -268,16 +275,20 @@ public class SingleInstanceEngineProvider implements AdblockEngineProvider
 
   private void disposeAdblock()
   {
-    Log.w(TAG, "Disposing adblock engine");
+    Log.w(TAG, "Waiting for lock");
+    synchronized (engineLock)
+    {
+      Log.w(TAG, "Disposing adblock engine");
 
-    engine.dispose();
-    engine = null;
+      engine.dispose();
+      engine = null;
 
-    // sometimes we need to deinit something after AdblockEngine instance disposed
-    // eg. release user settings
-    for (EngineDisposedListener listener : engineDisposedListeners)
-    {
-      listener.onAdblockEngineDisposed();
+      // sometimes we need to deinit something after AdblockEngine instance disposed
+      // eg. release user settings
+      for (EngineDisposedListener listener : engineDisposedListeners)
+      {
+        listener.onAdblockEngineDisposed();
+      }
     }
   }
 
@@ -286,4 +297,10 @@ public class SingleInstanceEngineProvider implements AdblockEngineProvider
   {
     return referenceCounter.get();
   }
+
+  @Override
+  public Object getEngineLock()
+  {
+    return engineLock;
+  }
 }