[SLIDER-1156] OutstandingRequestTracker edits map while iterating over its entries

• Type: Bug
• Status: Resolved
• Priority: Major
• Resolution: Fixed
• Affects Version/s: Slider 0.91
• Fix Version/s: Slider 0.92
• Component/s: other
• Assignee: Steve Loughran
• Reporter: Gour Saha
• Created: 15/Jul/16 07:52
• Updated: 06/Mar/17 21:23
• Resolved: 18/Jul/16 09:36

Description

The following error report is provided by coverity scan report on below method in OutstandingRequestTracker.java -

398  @SuppressWarnings("SynchronizationOnLocalVariableOrMethodParameter")
399  public synchronized List<AbstractRMOperation> cancelOutstandingAARequests() {
400
401    log.debug("Looking for AA request to cancel");
402    List<AbstractRMOperation> operations = new ArrayList<>();
403
404    // first, all placed requests
   	1. return_collection_alias: Call to entrySet returns an Iterable equivalent to placedRequests.
   	2. enhanced_for: Starting an iteration on placedRequests.entrySet().
   	3. Iterating over another element of placedRequests.entrySet()
   	
CID 120071 (#1 of 1): Using invalid iterator (INVALIDATE_ITERATOR)
7. invalid_loop: Attempting to obtain another element from placedRequests.entrySet() after it's been modified.
405    for (Map.Entry<RoleHostnamePair, OutstandingRequest> entry : placedRequests.entrySet()) {
406      OutstandingRequest outstandingRequest = entry.getValue();
407      synchronized (outstandingRequest) {
   	4. Condition outstandingRequest.isAntiAffine(), taking true branch
408        if (outstandingRequest.isAntiAffine()) {
409          // time to escalate
410          operations.add(outstandingRequest.createCancelOperation());
   	5. modify_iterable: Call to remove modifies Iterable placedRequests which invalidates the iterator for the loop on placedRequests.entrySet().
411          placedRequests.remove(entry.getKey());
412        }
413      }
   	6. Jumping back to the beginning of the loop
414    }
415    // second, all open requests
416    ListIterator<OutstandingRequest> orit = openRequests.listIterator();
417    while (orit.hasNext()) {
418      OutstandingRequest outstandingRequest =  orit.next();
419      synchronized (outstandingRequest) {
420        if (outstandingRequest.isAntiAffine()) {
421          // time to escalate
422          operations.add(outstandingRequest.createCancelOperation());
423          orit.remove();
424        }
425      }
426    }
427    log.info("Cancelling {} outstanding AA requests", operations.size());
428
429    return operations;
430  }