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

Side by Side 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.
Left:
Right:
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View unified diff | Download patch
OLDNEW
1 /* 1 /*
2 * This file is part of Adblock Plus <https://adblockplus.org/>, 2 * This file is part of Adblock Plus <https://adblockplus.org/>,
3 * Copyright (C) 2006-present eyeo GmbH 3 * Copyright (C) 2006-present eyeo GmbH
4 * 4 *
5 * Adblock Plus is free software: you can redistribute it and/or modify 5 * Adblock Plus is free software: you can redistribute it and/or modify
6 * it under the terms of the GNU General Public License version 3 as 6 * it under the terms of the GNU General Public License version 3 as
7 * published by the Free Software Foundation. 7 * published by the Free Software Foundation.
8 * 8 *
9 * Adblock Plus is distributed in the hope that it will be useful, 9 * Adblock Plus is distributed in the hope that it will be useful,
10 * but WITHOUT ANY WARRANTY; without even the implied warranty of 10 * but WITHOUT ANY WARRANTY; without even the implied warranty of
(...skipping 40 matching lines...) Expand 10 before | Expand all | Expand 10 after
51 private boolean developmentBuild; 51 private boolean developmentBuild;
52 private String preloadedPreferenceName; 52 private String preloadedPreferenceName;
53 private Map<String, Integer> urlToResourceIdMap; 53 private Map<String, Integer> urlToResourceIdMap;
54 private AdblockEngine engine; 54 private AdblockEngine engine;
55 private CountDownLatch engineCreated; 55 private CountDownLatch engineCreated;
56 private Long v8IsolateProviderPtr; 56 private Long v8IsolateProviderPtr;
57 private List<EngineCreatedListener> engineCreatedListeners = 57 private List<EngineCreatedListener> engineCreatedListeners =
58 new LinkedList<EngineCreatedListener>(); 58 new LinkedList<EngineCreatedListener>();
59 private List<EngineDisposedListener> engineDisposedListeners = 59 private List<EngineDisposedListener> engineDisposedListeners =
60 new LinkedList<EngineDisposedListener>(); 60 new LinkedList<EngineDisposedListener>();
61 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
61 62
62 /* 63 /*
63 Simple ARC management for AdblockEngine 64 Simple ARC management for AdblockEngine
64 Use `retain` and `release` 65 Use `retain` and `release`
65 */ 66 */
66 67
67 private AtomicInteger referenceCounter = new AtomicInteger(0); 68 private AtomicInteger referenceCounter = new AtomicInteger(0);
68 69
69 /** 70 /**
70 * Init with context 71 * Init with context
(...skipping 59 matching lines...) Expand 10 before | Expand all | Expand 10 after
130 this.engineDisposedListeners.remove(listener); 131 this.engineDisposedListeners.remove(listener);
131 } 132 }
132 133
133 public void clearEngineDisposedListeners() 134 public void clearEngineDisposedListeners()
134 { 135 {
135 this.engineDisposedListeners.clear(); 136 this.engineDisposedListeners.clear();
136 } 137 }
137 138
138 private void createAdblock() 139 private void createAdblock()
139 { 140 {
140 ConnectivityManager connectivityManager = 141 Log.w(TAG, "Waiting for lock");
141 (ConnectivityManager) context.getSystemService(Context.CONNECTIVITY_SERVIC E); 142 synchronized (engineLock)
142 IsAllowedConnectionCallback isAllowedConnectionCallback = 143 {
143 new IsAllowedConnectionCallbackImpl(connectivityManager); 144 Log.d(TAG, "Creating adblock engine ...");
145 ConnectivityManager connectivityManager =
146 (ConnectivityManager) context.getSystemService(Context.CONNECTIVITY_SERV ICE);
147 IsAllowedConnectionCallback isAllowedConnectionCallback =
148 new IsAllowedConnectionCallbackImpl(connectivityManager);
144 149
145 Log.d(TAG, "Creating adblock engine ..."); 150 AdblockEngine.Builder builder = AdblockEngine
151 .builder(
152 AdblockEngine.generateAppInfo(context, developmentBuild),
153 basePath)
154 .setIsAllowedConnectionCallback(isAllowedConnectionCallback)
155 .enableElementHiding(true);
146 156
147 AdblockEngine.Builder builder = AdblockEngine 157 if (v8IsolateProviderPtr != null)
148 .builder( 158 {
149 AdblockEngine.generateAppInfo(context, developmentBuild), 159 builder.useV8IsolateProvider(v8IsolateProviderPtr);
150 basePath) 160 }
151 .setIsAllowedConnectionCallback(isAllowedConnectionCallback)
152 .enableElementHiding(true);
153 161
154 if (v8IsolateProviderPtr != null) 162 // if preloaded subscriptions provided
155 { 163 if (preloadedPreferenceName != null)
156 builder.useV8IsolateProvider(v8IsolateProviderPtr); 164 {
157 } 165 SharedPreferences preloadedSubscriptionsPrefs = context.getSharedPrefere nces(
166 preloadedPreferenceName,
167 Context.MODE_PRIVATE);
168 builder.preloadSubscriptions(
169 context,
170 urlToResourceIdMap,
171 new AndroidWebRequestResourceWrapper.SharedPrefsStorage(preloadedSubsc riptionsPrefs));
172 }
158 173
159 // if preloaded subscriptions provided 174 engine = builder.build();
160 if (preloadedPreferenceName != null)
161 {
162 SharedPreferences preloadedSubscriptionsPrefs = context.getSharedPreferenc es(
163 preloadedPreferenceName,
164 Context.MODE_PRIVATE);
165 builder.preloadSubscriptions(
166 context,
167 urlToResourceIdMap,
168 new AndroidWebRequestResourceWrapper.SharedPrefsStorage(preloadedSubscri ptionsPrefs));
169 }
170 175
171 engine = builder.build(); 176 Log.d(TAG, "AdblockHelper engine created");
172 177
173 Log.d(TAG, "AdblockHelper engine created"); 178 // sometimes we need to init AdblockEngine instance, eg. set user settings
174 179 for (EngineCreatedListener listener : engineCreatedListeners)
175 // sometimes we need to init AdblockEngine instance, eg. set user settings 180 {
176 for (EngineCreatedListener listener : engineCreatedListeners) 181 listener.onAdblockEngineCreated(engine);
177 { 182 }
178 listener.onAdblockEngineCreated(engine);
179 } 183 }
180 } 184 }
181 185
182 @Override 186 @Override
183 public synchronized boolean retain(boolean asynchronous) 187 public synchronized boolean retain(boolean asynchronous)
184 { 188 {
185 boolean firstInstance = false; 189 boolean firstInstance = false;
186 190
187 if (referenceCounter.getAndIncrement() == 0) 191 if (referenceCounter.getAndIncrement() == 0)
188 { 192 {
189 firstInstance = true; 193 firstInstance = true;
190 194
191 if (!asynchronous) 195 if (!asynchronous)
192 { 196 {
193 createAdblock(); 197 createAdblock();
194 } 198 }
195 else 199 else
196 { 200 {
197 // latch is required for async (see `waitForReady()`) 201 // latch is required for async (see `waitForReady()`)
198 engineCreated = new CountDownLatch(1); 202 engineCreated = new CountDownLatch(1);
199 203
200 new Thread(new Runnable() 204 new Thread(new Runnable()
201 { 205 {
202 @Override 206 @Override
203 public void run() 207 public void run()
204 { 208 {
205 createAdblock(); 209 synchronized (engineLock)
210 {
211 createAdblock();
206 212
207 // unlock waiting client thread 213 // unlock waiting client thread
208 engineCreated.countDown(); 214 engineCreated.countDown();
215 }
209 } 216 }
210 }).start(); 217 }).start();
211 } 218 }
212 } 219 }
213 return firstInstance; 220 return firstInstance;
214 } 221 }
215 222
216 @Override 223 @Override
217 public void waitForReady() 224 public void waitForReady()
218 { 225 {
(...skipping 42 matching lines...) Expand 10 before | Expand all | Expand 10 after
261 else 268 else
262 { 269 {
263 disposeAdblock(); 270 disposeAdblock();
264 } 271 }
265 } 272 }
266 return lastInstance; 273 return lastInstance;
267 } 274 }
268 275
269 private void disposeAdblock() 276 private void disposeAdblock()
270 { 277 {
271 Log.w(TAG, "Disposing adblock engine"); 278 Log.w(TAG, "Waiting for lock");
279 synchronized (engineLock)
280 {
281 Log.w(TAG, "Disposing adblock engine");
272 282
273 engine.dispose(); 283 engine.dispose();
274 engine = null; 284 engine = null;
275 285
276 // sometimes we need to deinit something after AdblockEngine instance dispos ed 286 // sometimes we need to deinit something after AdblockEngine instance disp osed
277 // eg. release user settings 287 // eg. release user settings
278 for (EngineDisposedListener listener : engineDisposedListeners) 288 for (EngineDisposedListener listener : engineDisposedListeners)
279 { 289 {
280 listener.onAdblockEngineDisposed(); 290 listener.onAdblockEngineDisposed();
291 }
281 } 292 }
282 } 293 }
283 294
284 @Override 295 @Override
285 public int getCounter() 296 public int getCounter()
286 { 297 {
287 return referenceCounter.get(); 298 return referenceCounter.get();
288 } 299 }
300
301 @Override
302 public Object getEngineLock()
303 {
304 return engineLock;
305 }
289 } 306 }
OLDNEW

Powered by Google App Engine
This is Rietveld