Skip to content

Commit

Permalink
Avoid short circuiting when removing requests in RequestTracker
Browse files Browse the repository at this point in the history
Requests can be in both requests and pendingRequests. If we remove the
request from requests and the request is in pendingRequests, the item is
not removed from pendingRequests. If we attempt to clear the Request a
second time it will be found in pendingRequests and will be recycled a
second time, causing a crash.
  • Loading branch information
Sam Judd authored and sjudd committed Jun 15, 2017
1 parent 41cc06c commit 243c28c
Showing 1 changed file with 7 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,7 @@ public class RequestTracker {
Collections.newSetFromMap(new WeakHashMap<Request, Boolean>());
// A set of requests that have not completed and are queued to be run again. We use this list to
// maintain hard references to these requests to ensure that they are not garbage collected
// before
// they start running or while they are paused. See #346.
// before they start running or while they are paused. See #346.
@SuppressWarnings("MismatchedQueryAndUpdateOfCollection")
private final List<Request> pendingRequests = new ArrayList<>();
private boolean isPaused;
Expand All @@ -53,8 +52,12 @@ void addRequest(Request request) {
* request was removed or {@code false} if the request was not found.
*/
public boolean clearRemoveAndRecycle(Request request) {
boolean isOwnedByUs =
request != null && (requests.remove(request) || pendingRequests.remove(request));
if (request == null) {
return false;
}
boolean isOwnedByUs = requests.remove(request);
// Avoid short circuiting.
isOwnedByUs = pendingRequests.remove(request) || isOwnedByUs;
if (isOwnedByUs) {
request.clear();
request.recycle();
Expand Down

0 comments on commit 243c28c

Please sign in to comment.