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

Rylern · 2026-01-20T13:52:30Z

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:

qupath/qupath-extension-bioformats/src/main/java/qupath/lib/images/servers/bioformats/BioFormatsImageServer.java

Lines 1638 to 1651 in 837d984

    
           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():

qupath/qupath-extension-bioformats/src/main/java/qupath/lib/images/servers/bioformats/BioFormatsImageServer.java

Lines 1424 to 1425 in 837d984

    
           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:

qupath/qupath-extension-bioformats/src/main/java/qupath/lib/images/servers/bioformats/BioFormatsImageServer.java

Line 1439 in 837d984

    
           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:

qupath/qupath-extension-bioformats/src/main/java/qupath/lib/images/servers/bioformats/BioFormatsImageServer.java

Lines 1424 to 1425 in 837d984

    
           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.

petebankhead · 2026-01-20T17:31:11Z

I think this makes sense, so will merge and see how it behaves.

Would it be worthwhile to synchronise any use of the cleanables list?

Rylern · 2026-01-21T08:36:12Z

Would it be worthwhile to synchronise any use of the cleanables list?

In the scenario described above, that would prevent the ConcurrentModificationException. However, this would mean that a reader of the BioFormatsImageServer would not be closed after BioFormatsImageServer is closed, which is not ideal.

Return reader only if pool not closed

e41b838

Rylern requested a review from petebankhead January 20, 2026 13:52

petebankhead added this to the v0.7.0 milestone Jan 20, 2026

petebankhead merged commit 9b9bf59 into qupath:main Jan 20, 2026
3 checks passed

Rylern deleted the intermittent-concurrent-modification-exception branch January 21, 2026 08:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Return reader only if pool not closed#2074

Return reader only if pool not closed#2074
petebankhead merged 1 commit intoqupath:mainfrom
Rylern:intermittent-concurrent-modification-exception

Rylern commented Jan 20, 2026

Uh oh!

Uh oh!

petebankhead commented Jan 20, 2026

Uh oh!

Rylern commented Jan 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	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);
	}
	}
	}

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

Conversation

Rylern commented Jan 20, 2026

Uh oh!

Uh oh!

petebankhead commented Jan 20, 2026

Uh oh!

Rylern commented Jan 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants