Skip to content

Commit

Permalink
Fix MergingIterator.close() method to avoid ConcurrentModificationExc…
Browse files Browse the repository at this point in the history
…eption (#1056)
  • Loading branch information
cwhelan authored and tfenne committed Dec 13, 2017
1 parent 8296466 commit f45c654
Show file tree
Hide file tree
Showing 2 changed files with 41 additions and 3 deletions.
8 changes: 5 additions & 3 deletions src/main/java/htsjdk/samtools/util/MergingIterator.java
Original file line number Diff line number Diff line change
Expand Up @@ -132,9 +132,11 @@ public void remove() {
*/
@Override
public void close() {
for (final ComparableIterator iterator : this.queue) {
iterator.close();
this.queue.remove(iterator);
final Iterator<ComparableIterator> iterator = this.queue.iterator();
while (iterator.hasNext()) {
final ComparableIterator subIterator = iterator.next();
subIterator.close();
iterator.remove();
}
}

Expand Down
36 changes: 36 additions & 0 deletions src/test/java/htsjdk/samtools/util/MergingIteratorTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -173,4 +173,40 @@ public void testOutOfOrderIterators() {
Assert.assertEquals(mergingIterator.next().intValue(), 4);
mergingIterator.next(); // fails, because the next element would be "2"
}

@Test()
public void testCloseMultipleIteratorsMidIteration() {
final Queue<Integer> queueOne = new LinkedList<Integer>();
queueOne.add(1);
queueOne.add(4);
queueOne.add(7);

final Queue<Integer> queueTwo = new LinkedList<Integer>();
queueTwo.add(2);
queueTwo.add(5);
queueTwo.add(8);

final Queue<Integer> queueThree = new LinkedList<Integer>();
queueThree.add(3);
queueThree.add(6);
queueThree.add(9);

final Collection<CloseableIterator<Integer>> iterators = new ArrayList<CloseableIterator<Integer>>(3);
Collections.addAll(
iterators,
new QueueBackedIterator<>(queueOne),
new QueueBackedIterator<>(queueTwo),
new QueueBackedIterator<>(queueThree));

final MergingIterator<Integer> mergingIterator = new MergingIterator<Integer>(
INTEGER_COMPARATOR,
iterators);

Assert.assertEquals(mergingIterator.next().intValue(), 1);
Assert.assertEquals(mergingIterator.next().intValue(), 2);
Assert.assertEquals(mergingIterator.next().intValue(), 3);

mergingIterator.close();
Assert.assertFalse(mergingIterator.hasNext());
}
}

0 comments on commit f45c654

Please sign in to comment.