Ticket #1606 (closed defect: fixed)

Opened 6 years ago

Last modified 6 years ago

Access ObjectService in a thread safe manner

Reported by: bdezonia Owned by: bdezonia
Priority: major Milestone: imagej2-b7-ndim-data
Component: Core Version:
Severity: serious Keywords:
Cc: Blocked By:
Blocking: #1398

Description

The ObjectService has a get(ClassType t) method that returns an unmodifiable List of the objects of the given type. However code in other threads elsewhere can be modifying the data underlying the unmodifiable list. This can result in exceptions being thrown.

We should make changes in our code to make sure that the ObjectService is handled in a thread safe manner. Either by using synchronized code or by copying data judiciously.

The current example for duplicating the problem relies on code that currently only exists in the legacy-tracker branch. If IJ2 built from that code (or if that code has been merged into master) the issue can be replicated by opening IJ2 and then holding down Shift Command B until IJ2 opens 30-40 copies of Blobs. Under Eclipse the console window will have 1 or more copies of an exception in it:

Uncaught exception in thread Thread[IJ1 legacy thread,6,IJ1 legacy group]
java.util.ConcurrentModificationException

at java.util.AbstractList$Itr.checkForComodification(AbstractList.java:372)
at java.util.AbstractList$Itr.next(AbstractList.java:343)
at java.util.Collections$UnmodifiableCollection$1.next(Collections.java:1008)
at imagej.legacy.plugin.LegacyCommand$LegacyCommandThread.updateImagePlusesFromDisplays(LegacyCommand.java:338)
at imagej.legacy.plugin.LegacyCommand$LegacyCommandThread.run(LegacyCommand.java:183)

Change History

comment:1 Changed 6 years ago by bdezonia

  • Blocking 1398 added

comment:2 Changed 6 years ago by bdezonia

Note this issue is different from #1605

comment:3 Changed 6 years ago by bdezonia

Tried two approaches pushed as branches object-index-broken and object-index-working. The broken version relies on synchronized methods in DefaultObjectService and also the get() method in there makes data copies. The working version simply makes copies in ObjectIndex itself.

To test behavior open IJ2 and hold Shift Cmd B down to make many many Blobs images. In the broken version you can get two exceptions: one in ByteProcessor which is a different issue (#1605) and the bad one which is a concurrent modification exception. In the working version you only get the ByteProcessor issue. In both versions the display titles can be wrong. Note that the working version opens the images more quickly than the broken version.

comment:4 Changed 6 years ago by bdezonia

Note BTW that the legacy-tracker branch was merged to master on 1-7-13. Holding down Shift Command B until many Blobs opened should generate exceptions as described here and in #1605.

comment:5 Changed 6 years ago by bdezonia

  • Status changed from new to reviewing

CTR please review the code in object-index-broken and object-index-working branches. You were strongly in the object-index-broken camp when I made these branches.

comment:6 Changed 6 years ago by curtis

  • Owner changed from curtis to bdezonia

After looking at the branches with fresh eyes, I now prefer the object-index-working branch. It is much simpler, and the overhead of creating a copy each time is not really too bad. So have  pushed the change to SciJava Common. Please test and close this ticket if you believe the problem is resolved.

comment:7 Changed 6 years ago by bdezonia

  • Status changed from reviewing to closed
  • Resolution set to fixed

Tested and it appears to be fixed.

Note: See TracTickets for help on using tickets.