Issue 4591 - Crash after dispose

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

Index: libadblockplus-android/src/org/adblockplus/libadblockplus/android/AdblockEngine.java
diff --git a/libadblockplus-android/src/org/adblockplus/libadblockplus/android/AdblockEngine.java b/libadblockplus-android/src/org/adblockplus/libadblockplus/android/AdblockEngine.java
index 1046b73471026950f83c0915942215a8f1052be2..83ca9d79859343058aef4a4dfc29f4d0fe9497c4 100644
--- a/libadblockplus-android/src/org/adblockplus/libadblockplus/android/AdblockEngine.java
+++ b/libadblockplus-android/src/org/adblockplus/libadblockplus/android/AdblockEngine.java
@@ -103,46 +103,6 @@ public final class AdblockEngine
         .build();
   }
 
-  public static final UpdateAvailableCallback UPDATE_AVAILABLE_CALLBACK =
-    new UpdateAvailableCallback()
-  {
-    @Override
-    public void updateAvailableCallback(String url)
-    {
-      Log.d(TAG, "Update available for " + url);
-    }
-  };
-
-  public static final UpdateCheckDoneCallback UPDATE_CHECK_DONE_CALLBACK =
-    new UpdateCheckDoneCallback()
-  {
-    @Override
-    public void updateCheckDoneCallback(String error)
-    {
-      Log.d(TAG, "Update check done, error: " + error);
-    }
-  };
-
-  public static final ShowNotificationCallback SHOW_NOTIFICATION_CALLBACK =
-    new ShowNotificationCallback()
-  {
-    @Override
-    public void showNotificationCallback(Notification jsValue)
-    {
-      Log.d(TAG, "Notification: " + jsValue);
-    }
-  };
-
-  public static final FilterChangeCallback FILTER_CHANGE_CALLBACK =
-    new FilterChangeCallback()
-  {
-    @Override
-    public void filterChangeCallback(String action, JsValue jsValue)
-    {
-      Log.d(TAG, "Filter changed: " + action + (!jsValue.isUndefined() ? ", " + jsValue : ""));
-    }
-  };
-
   public static AdblockEngine create(final Context context, final AppInfo appInfo,
                                      final String basePath, boolean enableElemhide,
                                      UpdateAvailableCallback updateAvailableCallback,
@@ -193,28 +153,13 @@ public final class AdblockEngine
   public static AdblockEngine create(final Context context, final AppInfo appInfo,
                                      final String basePath, boolean elemhideEnabled)
   {
-    return create(context, appInfo, basePath, elemhideEnabled,
-      UPDATE_AVAILABLE_CALLBACK, UPDATE_CHECK_DONE_CALLBACK,
-      SHOW_NOTIFICATION_CALLBACK, FILTER_CHANGE_CALLBACK);
+    return create(context, appInfo, basePath, elemhideEnabled, null, null, null, null);

[diegocarloslima, 2016/11/04 10:35:15]

Is this callback cleanup related to the crash? If not, It might be better to do
it in a separate issue 

[anton, 2016/11/07 07:35:14]

Yes, it's related as since we use smart pointers callback is fired after we no
longer need it and it causes crash since vm is released (GetJavaVM() returns
NULL
(https://hg.adblockplus.org/libadblockplus-android/file/tip/libadblockplus-and...)
and cause NRE with sigsegv 11 (see issue log attachments)).

[Felix Dahlke, 2016/11/18 06:56:55]

So what if the user passes these callback objects in? Wouldn't we then have the
exact same problem as with our static versions? (Not that we seem to need those
for anything but logging.)

   }
 
   public void dispose()
   {
     Log.w(TAG, "Dispose");
 
-    // Safe disposing (just in case)
-    if (this.filterEngine != null)
-    {
-      this.filterEngine.dispose();
-      this.filterEngine = null;
-    }
-
-    if (this.jsEngine != null)
-    {
-      this.jsEngine.dispose();
-      this.jsEngine = null;
-    }
-
     if (this.logSystem != null)
     {
       this.logSystem.dispose();
@@ -229,6 +174,11 @@ public final class AdblockEngine
 
     if (this.updateAvailableCallback != null)
     {
+      if (this.filterEngine != null)
+      {
+        this.filterEngine.removeUpdateAvailableCallback();
+      }
+
       this.updateAvailableCallback.dispose();
       this.updateAvailableCallback = null;
     }
@@ -241,15 +191,38 @@ public final class AdblockEngine
 
     if (this.filterChangeCallback != null)
     {
+      if (this.filterEngine != null)
+      {
+        this.filterEngine.removeFilterChangeCallback();
+      }
+
       this.filterChangeCallback.dispose();
       this.filterChangeCallback = null;
     }
 
     if (this.showNotificationCallback != null)
     {
+      if (this.filterEngine != null)
+      {
+        this.filterEngine.removeShowNotificationCallback();
+      }
+
       this.showNotificationCallback.dispose();
       this.showNotificationCallback = null;
     }
+
+    // Safe disposing (just in case)
+    if (this.filterEngine != null)
+    {
+      this.filterEngine.dispose();
+      this.filterEngine = null;
+    }
+
+    if (this.jsEngine != null)
+    {
+      this.jsEngine.dispose();
+      this.jsEngine = null;
+    }
   }
 
   public boolean isFirstRun()