From 3131b188f0fa10738a4ba11095651f465c0e8b18 Mon Sep 17 00:00:00 2001 From: Sean Barbeau Date: Thu, 19 Mar 2020 10:11:07 -0400 Subject: [PATCH] fix: Revert "Use thread pools (#601)" This reverts commit 6619f14c. It seems that when a thread pool is used it allows a thread to try and unlock() a lock that it did not lock(), which ReentrantLock does not allow - "If the current thread is not the holder of this lock then IllegalMonitorStateException is thrown." Fixes #660 --- .../algo/PreCachingAlgorithmDecorator.java | 13 +++--- .../view/DefaultClusterRenderer.java | 42 +++++++++---------- 2 files changed, 26 insertions(+), 29 deletions(-) diff --git a/library/src/main/java/com/google/maps/android/clustering/algo/PreCachingAlgorithmDecorator.java b/library/src/main/java/com/google/maps/android/clustering/algo/PreCachingAlgorithmDecorator.java index 0a7032e85..a39106a68 100644 --- a/library/src/main/java/com/google/maps/android/clustering/algo/PreCachingAlgorithmDecorator.java +++ b/library/src/main/java/com/google/maps/android/clustering/algo/PreCachingAlgorithmDecorator.java @@ -16,18 +16,16 @@ package com.google.maps.android.clustering.algo; +import androidx.collection.LruCache; + import com.google.maps.android.clustering.Cluster; import com.google.maps.android.clustering.ClusterItem; import java.util.Collection; import java.util.Set; -import java.util.concurrent.Executor; -import java.util.concurrent.Executors; import java.util.concurrent.locks.ReadWriteLock; import java.util.concurrent.locks.ReentrantReadWriteLock; -import androidx.collection.LruCache; - /** * Optimistically fetch clusters for adjacent zoom levels, caching them as necessary. */ @@ -37,7 +35,6 @@ public class PreCachingAlgorithmDecorator extends Abstrac // TODO: evaluate maxSize parameter for LruCache. private final LruCache>> mCache = new LruCache>>(5); private final ReadWriteLock mCacheLock = new ReentrantReadWriteLock(); - private final Executor mExecutor = Executors.newCachedThreadPool(); public PreCachingAlgorithmDecorator(Algorithm algorithm) { mAlgorithm = algorithm; @@ -104,10 +101,12 @@ public Set> getClusters(float zoom) { Set> results = getClustersInternal(discreteZoom); // TODO: Check if requests are already in-flight. if (mCache.get(discreteZoom + 1) == null) { - mExecutor.execute(new PrecacheRunnable(discreteZoom + 1)); + // It seems this cannot use a thread pool due to thread locking issues (#660) + new Thread(new PrecacheRunnable(discreteZoom + 1)).start(); } if (mCache.get(discreteZoom - 1) == null) { - mExecutor.execute(new PrecacheRunnable(discreteZoom - 1)); + // It seems this cannot use a thread pool due to thread locking issues (#660) + new Thread(new PrecacheRunnable(discreteZoom - 1)).start(); } return results; } diff --git a/library/src/main/java/com/google/maps/android/clustering/view/DefaultClusterRenderer.java b/library/src/main/java/com/google/maps/android/clustering/view/DefaultClusterRenderer.java index 75f6be047..024405cc7 100644 --- a/library/src/main/java/com/google/maps/android/clustering/view/DefaultClusterRenderer.java +++ b/library/src/main/java/com/google/maps/android/clustering/view/DefaultClusterRenderer.java @@ -16,24 +16,6 @@ package com.google.maps.android.clustering.view; -import com.google.android.gms.maps.GoogleMap; -import com.google.android.gms.maps.Projection; -import com.google.android.gms.maps.model.BitmapDescriptor; -import com.google.android.gms.maps.model.BitmapDescriptorFactory; -import com.google.android.gms.maps.model.LatLng; -import com.google.android.gms.maps.model.LatLngBounds; -import com.google.android.gms.maps.model.Marker; -import com.google.android.gms.maps.model.MarkerOptions; -import com.google.maps.android.R; -import com.google.maps.android.clustering.Cluster; -import com.google.maps.android.clustering.ClusterItem; -import com.google.maps.android.clustering.ClusterManager; -import com.google.maps.android.collections.MarkerManager; -import com.google.maps.android.geometry.Point; -import com.google.maps.android.projection.SphericalMercatorProjection; -import com.google.maps.android.ui.IconGenerator; -import com.google.maps.android.ui.SquareTextView; - import android.animation.Animator; import android.animation.AnimatorListenerAdapter; import android.animation.TimeInterpolator; @@ -55,6 +37,24 @@ import android.view.ViewGroup; import android.view.animation.DecelerateInterpolator; +import com.google.android.gms.maps.GoogleMap; +import com.google.android.gms.maps.Projection; +import com.google.android.gms.maps.model.BitmapDescriptor; +import com.google.android.gms.maps.model.BitmapDescriptorFactory; +import com.google.android.gms.maps.model.LatLng; +import com.google.android.gms.maps.model.LatLngBounds; +import com.google.android.gms.maps.model.Marker; +import com.google.android.gms.maps.model.MarkerOptions; +import com.google.maps.android.R; +import com.google.maps.android.clustering.Cluster; +import com.google.maps.android.clustering.ClusterItem; +import com.google.maps.android.clustering.ClusterManager; +import com.google.maps.android.collections.MarkerManager; +import com.google.maps.android.geometry.Point; +import com.google.maps.android.projection.SphericalMercatorProjection; +import com.google.maps.android.ui.IconGenerator; +import com.google.maps.android.ui.SquareTextView; + import java.util.ArrayList; import java.util.Collections; import java.util.HashMap; @@ -64,8 +64,6 @@ import java.util.Queue; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.Executor; -import java.util.concurrent.Executors; import java.util.concurrent.locks.Condition; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantLock; @@ -79,7 +77,6 @@ public class DefaultClusterRenderer implements ClusterRen private final ClusterManager mClusterManager; private final float mDensity; private boolean mAnimate; - private final Executor mExecutor = Executors.newSingleThreadExecutor(); private static final int[] BUCKETS = {10, 20, 50, 100, 200, 500, 1000}; private ShapeDrawable mColoredCircleBackground; @@ -292,7 +289,8 @@ public void run() { }); renderTask.setProjection(projection); renderTask.setMapZoom(mMap.getCameraPosition().zoom); - mExecutor.execute(renderTask); + // It seems this cannot use a thread pool due to thread locking issues (#660) + new Thread(renderTask).start(); } public void queue(Set> clusters) {