Home Original page

Return reader only if pool not closed by Rylern · Pull Request #2074 · qupath/qupath

Attempt to fix #2052

The exception of the issue looks like this:

java.util.ConcurrentModificationException
	at java.base/java.util.ArrayList$Itr.checkForComodification(ArrayList.java:1096)
	at java.base/java.util.ArrayList$Itr.next(ArrayList.java:1050)
	at qupath.lib.images.servers.bioformats.BioFormatsImageServer$ReaderPool.close(BioFormatsImageServer.java:1643)
	at qupath.lib.images.servers.bioformats.BioFormatsImageServer.close(BioFormatsImageServer.java:959)
	at qupath.lib.images.writers.ome.zarr.PyramidalOMEZarrWriter.writeImage(PyramidalOMEZarrWriter.java:165)
	at qupath.lib.images.writers.ome.zarr.TestPyramidalOMEZarrWriter.Check_Image_Pixels_With_Base_Server_With_Downsample_2(TestPyramidalOMEZarrWriter.java:168)
	at java.base/java.lang.reflect.Method.invoke(Method.java:565)
	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:186)
	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:214)
	at java.base/java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:197)
	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:214)
	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:186)
	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:214)
	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:186)
	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.accept(ForEachOps.java:186)
	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:214)
	at java.base/java.util.Spliterators$ArraySpliterator.forEachRemaining(Spliterators.java:1024)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:570)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:560)
	at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:153)
	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:176)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:265)
	at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:632)
	at java.base/java.util.stream.ReferencePipeline$7$1FlatMap.accept(ReferencePipeline.java:293)
	at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1716)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:570)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:560)
	at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:153)
	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:176)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:265)
	at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:632)
	at java.base/java.util.stream.ReferencePipeline$7$1FlatMap.accept(ReferencePipeline.java:293)
	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:214)
	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:214)
	at java.base/java.util.stream.ReferencePipeline$3$1.accept(ReferencePipeline.java:214)
	at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1716)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:570)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:560)
	at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:153)
	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:176)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:265)
	at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:632)
	at java.base/java.util.stream.ReferencePipeline$7$1FlatMap.accept(ReferencePipeline.java:293)
	at java.base/java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1716)
	at java.base/java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:570)
	at java.base/java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:560)
	at java.base/java.util.stream.ForEachOps$ForEachOp.evaluateSequential(ForEachOps.java:153)
	at java.base/java.util.stream.ForEachOps$ForEachOp$OfRef.evaluateSequential(ForEachOps.java:176)
	at java.base/java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:265)
	at java.base/java.util.stream.ReferencePipeline.forEach(ReferencePipeline.java:632)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1604)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1604)

This ConcurrentModificationException occurs while iterating on BioFormatsImageServer.ReaderPool.cleanables in the close() function:

public void close() throws Exception {
logger.debug("Closing ReaderManager");
isClosed = true;
if (task != null && !task.isDone())
task.cancel(true);
for (var c : cleanables) {
try {
c.clean();
} catch (Exception e) {
logger.error("Exception during cleanup: {}", e.getMessage(), e);
}
}
}

This means cleanables is modified somewhere else. The only possible place is in ReaderPool.createReader():

cleanables.add(cleaner.register(this,
new ReaderCleaner(Integer.toString(cleanables.size()+1), imageReader)));

This means that the BioFormatsImageServer is closed while a reader is being created. My theory is that this scenario occurs when:

  • Let's consider that the BioFormatsImageServer has 5 actives readers, which are all being used at the moment, and can create up to 10 readers.
  • The calling code calls BioFormatsImageServer.readTile(). Since all active readers are being used and it's possible to create a new reader, a request to create a new reader is made. This is done in a thread of the common pool:
task = ForkJoinPool.commonPool().submit(() -> createAdditionalReader(options, classList, id, args));
  • While the new reader is being created, one of the 5 readers becomes available. The calling code uses this reader to read the image.
  • Still while the new reader is being created, the calling code finishes reading the tile. Let's consider that it was the last tile to read. The calling code then closes the BioFormatsImageServer.
  • While the BioFormatsImageServer is being closed, the new reader of above finishes being created. A new cleanable is added:
cleanables.add(cleaner.register(this,
new ReaderCleaner(Integer.toString(cleanables.size()+1), imageReader)));

This triggers the ConcurrentModificationException.

This scenario is quite unlikely, which is why #2052 rarely happens. This PR attempt to fixes that by checking if the BioFormatsImageServer is closed before adding a new cleanable.

This doesn't totally fixes the issue (the ConcurrentModificationException could still happen), but it should now be very unlikely. I run the unit tests around 200 times with the GitHub actions, and the issue never happened again.