ClassUtils: Caching of annotated Fields is not thread safe

[dietzc]

@hinerm @ctrueden

Following example fails with error message: "Context already injected ..." (sorry for using SCIFIO to demonstrate a SciJava Bug):

public static void main(String[] args) {

    final SCIFIO scifio = new SCIFIO();

    final Runnable simpleRunnable = new Runnable() {

        @Override
        public void run() {
            new ImgOpener(scifio.getContext());

        }
    };

    new Thread(simpleRunnable).start();
    new Thread(simpleRunnable).start();

}

I was able to track the problem down to the caching of the annotated Fields of Plugins. The problem is, that the caching in e.g. https://github.com/scijava/scijava-common/blob/master/src/main/java/org/scijava/util/ClassUtils.java#L369 seems not to be thread safe. The problem that occurs with that is that the annotated Fields are added twice to the cached list (and therefore the Context tries to inject itself twice (see: https://github.com/scijava/scijava-common/blob/master/src/main/java/org/scijava/util/ClassUtils.java#L369) I think an easy fix would be to synchronize the accesses to the Cache in ClassUtils. However, you may want to be careful with that and only synchronize the really critical parts of the method (synchronize may harm performance).

Does this make sense?

[hinerm]

However, you may want to be careful with that and only synchronize the really critical parts of the method (synchronize may harm performance).

I decided not to be careful, and put everything in a giant synchronized block.

I documented several options for future development if this turns out to be a problem - but decided not to try implementing any of them proactively as they all have their own drawbacks, and/or wouldn't actually improve performance in your use case.

Thanks for the report + example @dietzc !

[dietzc]

thanks @hinerm. I will let you know if performance is an issue :-)

[ctrueden]

I think there are still bugs remaining surrounding the field cache and multithreaded access. Sometimes I see the following exception (repeatedly) when using the ImageJ search bar:

ImageJ search bar stack trace

Exception in thread "AWT-EventQueue-0" java.lang.NullPointerException
	at java.util.ArrayList.addAll(ArrayList.java:581)
	at org.scijava.util.ClassUtils.getAnnotatedFields(ClassUtils.java:196)
	at org.scijava.util.ClassUtils.getAnnotatedFields(ClassUtils.java:166)
	at org.scijava.command.CommandInfo.checkFields(CommandInfo.java:447)
	at org.scijava.command.CommandInfo.initParams(CommandInfo.java:433)
	at org.scijava.command.CommandInfo.parseParams(CommandInfo.java:428)
	at org.scijava.command.CommandInfo.inputs(CommandInfo.java:283)
	at org.scijava.batch.BatchService.supportsModule(BatchService.java:49)
	at org.scijava.batch.BatchModuleSearchActionFactory.supports(BatchModuleSearchActionFactory.java:53)
	at org.scijava.batch.BatchModuleSearchActionFactory.supports(BatchModuleSearchActionFactory.java:44)
	at org.scijava.search.SearchService.lambda$actions$0(SearchService.java:69)
	at java.util.stream.ReferencePipeline$2$1.accept(ReferencePipeline.java:174)
	at java.util.ArrayList$ArrayListSpliterator.forEachRemaining(ArrayList.java:1382)
	at java.util.stream.AbstractPipeline.copyInto(AbstractPipeline.java:481)
	at java.util.stream.AbstractPipeline.wrapAndCopyInto(AbstractPipeline.java:471)
	at java.util.stream.ReduceOps$ReduceOp.evaluateSequential(ReduceOps.java:708)
	at java.util.stream.AbstractPipeline.evaluate(AbstractPipeline.java:234)
	at java.util.stream.ReferencePipeline.collect(ReferencePipeline.java:499)
	at org.scijava.search.SearchService.actions(SearchService.java:71)
	at org.scijava.ui.swing.search.SwingSearchBar$SwingSearchPanel.lambda$new$6(SwingSearchBar.java:515)
	at javax.swing.JList.fireSelectionValueChanged(JList.java:1802)
	at javax.swing.JList$ListSelectionHandler.valueChanged(JList.java:1816)
	at javax.swing.DefaultListSelectionModel.fireValueChanged(DefaultListSelectionModel.java:184)
	at javax.swing.DefaultListSelectionModel.fireValueChanged(DefaultListSelectionModel.java:164)
	at javax.swing.DefaultListSelectionModel.fireValueChanged(DefaultListSelectionModel.java:211)
	at javax.swing.DefaultListSelectionModel.changeSelection(DefaultListSelectionModel.java:405)
	at javax.swing.DefaultListSelectionModel.changeSelection(DefaultListSelectionModel.java:415)
	at javax.swing.DefaultListSelectionModel.setSelectionInterval(DefaultListSelectionModel.java:459)
	at javax.swing.JList.setSelectedIndex(JList.java:2216)
	at org.scijava.ui.swing.search.SwingSearchBar$SwingSearchPanel.rebuild(SwingSearchBar.java:673)
	at org.scijava.ui.swing.search.SwingSearchBar$SwingSearchPanel.update(SwingSearchBar.java:570)
	at org.scijava.ui.swing.search.SwingSearchBar$SwingSearchPanel.lambda$null$0(SwingSearchBar.java:341)
	at org.scijava.thread.DefaultThreadService$2.run(DefaultThreadService.java:221)
	at java.awt.event.InvocationEvent.dispatch(InvocationEvent.java:311)
	at java.awt.EventQueue.dispatchEventImpl(EventQueue.java:758)
	at java.awt.EventQueue.access$500(EventQueue.java:97)
	at java.awt.EventQueue$3.run(EventQueue.java:709)
	at java.awt.EventQueue$3.run(EventQueue.java:703)
	at java.security.AccessController.doPrivileged(Native Method)
	at java.security.ProtectionDomain$JavaSecurityAccessImpl.doIntersectionPrivilege(ProtectionDomain.java:74)
	at java.awt.EventQueue.dispatchEvent(EventQueue.java:728)
	at java.awt.EventDispatchThread.pumpOneEventForFilters(EventDispatchThread.java:205)
	at java.awt.EventDispatchThread.pumpEventsForFilter(EventDispatchThread.java:116)
	at java.awt.EventDispatchThread.pumpEventsForHierarchy(EventDispatchThread.java:105)
	at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:101)
	at java.awt.EventDispatchThread.pumpEvents(EventDispatchThread.java:93)
	at java.awt.EventDispatchThread.run(EventDispatchThread.java:82)