Skip to content

Conversation

@barbeau
Copy link
Collaborator

@barbeau barbeau commented Mar 19, 2020

This reverts commit 6619f14 (and I added comments to avoid someone using a thread pool again in the future).

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."

I can't reproduce #660 myself, but based on the description and stack trace in #660 it makes sense that this change could cause this issue in certain multi-threaded states.

It would be good to get a release out in the near term with this fix to see if it does indeed resolve the issue.

Fixes #660

This reverts commit 6619f14. 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
@barbeau barbeau requested a review from arriolac March 19, 2020 14:16
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Mar 19, 2020
@codecov
Copy link

codecov bot commented Mar 19, 2020

Codecov Report

Merging #665 into master will increase coverage by 0.01%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #665      +/-   ##
==========================================
+ Coverage   39.33%   39.35%   +0.01%     
==========================================
  Files          71       71              
  Lines        4037     4035       -2     
  Branches      606      606              
==========================================
  Hits         1588     1588              
+ Misses       2346     2344       -2     
  Partials      103      103              
Impacted Files Coverage Δ
.../clustering/algo/PreCachingAlgorithmDecorator.java 0.00% <0.00%> (ø)
...ndroid/clustering/view/DefaultClusterRenderer.java 0.00% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8689d29...3131b18. Read the comment docs.

Copy link
Contributor

@arriolac arriolac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM :shipit:

@barbeau barbeau merged commit ace4909 into googlemaps:master Mar 19, 2020
@barbeau barbeau deleted the cluster-lock branch March 19, 2020 20:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

android-map-utils: Crash in ClusterTask

3 participants