ConcurrentModificationException on opening meta data editor

[henning-gerhardt]

On open a process in meta data editor with a lot of entries the following exception got logged:

15-Jun-2021 06:54:00.500 SEVERE [http-nio-127.0.0.1-8080-exec-3] org.apache.catalina.core.StandardWrapperValve.invoke Servlet.service() for servlet [Faces Servlet] in context with path [/kitodo] threw exception [null] with root cause
        java.util.ConcurrentModificationException
                at java.util.HashMap$HashIterator.nextNode(HashMap.java:1445)
                at java.util.HashMap$KeyIterator.next(HashMap.java:1469)
                at org.kitodo.production.forms.dataeditor.StructurePanel.buildStructureTreeRecursively(StructurePanel.java:470)
                at org.kitodo.production.forms.dataeditor.StructurePanel.orderChildrenAndViews(StructurePanel.java:538)
                at org.kitodo.production.forms.dataeditor.StructurePanel.buildStructureTreeRecursively(StructurePanel.java:501)
                at org.kitodo.production.forms.dataeditor.StructurePanel.orderChildrenAndViews(StructurePanel.java:538)
                at org.kitodo.production.forms.dataeditor.StructurePanel.buildStructureTreeRecursively(StructurePanel.java:501)
                at org.kitodo.production.forms.dataeditor.StructurePanel.orderChildrenAndViews(StructurePanel.java:538)
                at org.kitodo.production.forms.dataeditor.StructurePanel.buildStructureTreeRecursively(StructurePanel.java:501)
                at org.kitodo.production.forms.dataeditor.StructurePanel.buildStructureTree(StructurePanel.java:457)
                at org.kitodo.production.forms.dataeditor.StructurePanel.show(StructurePanel.java:420)
                at org.kitodo.production.forms.dataeditor.DataEditorForm.init(DataEditorForm.java:285)
                at org.kitodo.production.forms.dataeditor.DataEditorForm.open(DataEditorForm.java:252)
                at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
                at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
                at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
                at java.lang.reflect.Method.invoke(Method.java:498)
                at org.apache.el.parser.AstValue.invoke(AstValue.java:247)
                at org.apache.el.MethodExpressionImpl.invoke(MethodExpressionImpl.java:267)
                at org.jboss.weld.util.el.ForwardingMethodExpression.invoke(ForwardingMethodExpression.java:40)
                at org.jboss.weld.el.WeldMethodExpression.invoke(WeldMethodExpression.java:50)
                at org.jboss.weld.util.el.ForwardingMethodExpression.invoke(ForwardingMethodExpression.java:40)
                at org.jboss.weld.el.WeldMethodExpression.invoke(WeldMethodExpression.java:50)
                at org.apache.myfaces.view.facelets.el.ContextAwareTagMethodExpression.invoke(ContextAwareTagMethodExpression.java:96)
                at org.apache.myfaces.application.ActionListenerImpl.processAction(ActionListenerImpl.java:74)
                at org.springframework.faces.webflow.FlowActionListener.processAction(FlowActionListener.java:71)
                at org.springframework.faces.model.SelectionTrackingActionListener.processAction(SelectionTrackingActionListener.java:64)

....

It looks like that a non thread safe list (in this case a list of type ArrayList) is modified by two or more parallel executed threads.

[matthias-ronge]

Thanks for the report. Such bugs are difficult to catch.

The collection modified is the field HashSet<Process> currentChildren of DataEditorForm. It is populated in open(…) and cleared in close(…) and iterated over in StructurePanel.buildStructureTreeRecursively(…). I found only one more access to this collection, an add(…) operation in AddDocStrucTypeDialog.addLinkButtonClick(), when manually adding a link to another pocess. Could it be that one AJAX request is still building the page in the background (one thread) while another thread performs the add(…) operation? Other scenarios would be frightening if you have a process in the metadata editor open in a browser window that updates something via AJAX and you or in the worst case even another user opens a process in the metadata editor in another browser window at the same time.

[henning-gerhardt]

If such kind of error is already happening with only one user is logged in and did only try to load on big process with many entries what should happen if many users (> 15) try to load and work at the same time different processes in meta data editor?

[matthias-ronge]

You only found the stack trace in your log file, right? You don't know when exactly this happened, or do you have a process in which this happens reproducibly when opening (or with a high degree of probability)?

I would first check the function with which you can manually link a process as a child. Maybe the mistake happened there.

[henning-gerhardt]

So far as I know there was some editing including adding new document structure elements but I don't know the title / process.

Using non-thread safe lists / elements in a threading application is never good and hoping that a concurrent access did not happen is more then an optimistic assumption.

[matthias-ronge]

Making objects threadsafe creates additional work for the JVM. So generally it is not better to always implement all objects threadsafe, because all threadsafe system calls trigger an internal synchronization between the running threads, so it is a question of whether one has to afford this in one place. You shouldn't just do it, just because it can be done that way, I think. I think that was also a confusion in the early days of Java, when sometimes primitive classes where you wouldn't expect that, were implemented threadsafe (for example Vector and StringBuffer) and you have the feeling that it was only made to cover the case that it was needed. But many more classes don't do that, and maybe the idea was an over-generalization of the ancient Java developers. So we should approach the topic carefully, but of course, it has to solve bugs, but maybe it may areise new bugs, because in the render response phase Tomcat also relies on objects not to change, otherwise that is a problem afterwards because IDs do not match any longer.