Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Unified Diff: libadblockplus-android/src/org/adblockplus/libadblockplus/android/SingleInstanceEngineProvider.java

Issue 29691582: Issue 6366 - Elemhide thread is accessing released engine (Closed)
Patch Set: Created Feb. 7, 2018, 1:48 p.m.
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View side-by-side diff with in-line comments
Download patch
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;
+ }
}

Powered by Google App Engine
This is Rietveld