sameeraroshan / visjs

visjs vaadin addon
Apache License 2.0
10 stars 13 forks source link

ConcurrentModificationException on modifying graph on (click) event #7

Open evenbits opened 9 years ago

evenbits commented 9 years ago

I need to modify a graph as a result of a click event. So I register a listener for reacting to this event by using

addNodeSelectListener(new Node.NodeSelectListener(node) {
                @Override
                public void onFired(SelectEvent selectEvent) {
                    onSelect(selectEvent.getNodeIds());
                }
            });)

But if one of these listeners fires and within the same thread I add additional nodes to the graph and I (need? to) register additional listeners for these nodes, this results in a concurrentModificationException after the listener completes.

java.util.ConcurrentModificationException
    at java.util.ArrayList$Itr.checkForComodification(ArrayList.java:886)
    at java.util.ArrayList$Itr.next(ArrayList.java:836)
    at org.vaadin.visjs.networkDiagram.NetworkDiagram.fireNodeSelectEvent(NetworkDiagram.java:390)
    at org.vaadin.visjs.networkDiagram.NetworkDiagram$1.call(NetworkDiagram.java:47)
    at com.vaadin.server.JavaScriptCallbackHelper$1.call(JavaScriptCallbackHelper.java:81)
    at sun.reflect.GeneratedMethodAccessor197.invoke(Unknown Source)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:483)
    at com.vaadin.server.ServerRpcManager.applyInvocation(ServerRpcManager.java:168)
    at com.vaadin.server.ServerRpcManager.applyInvocation(ServerRpcManager.java:118)
    at com.vaadin.server.communication.ServerRpcHandler.handleInvocations(ServerRpcHandler.java:287)
    at com.vaadin.server.communication.ServerRpcHandler.handleRpc(ServerRpcHandler.java:180)
    at com.vaadin.server.communication.UidlRequestHandler.synchronizedHandleRequest(UidlRequestHandler.java:93)
    at com.vaadin.server.SynchronizedRequestHandler.handleRequest(SynchronizedRequestHandler.java:41)
    at com.vaadin.server.VaadinService.handleRequest(VaadinService.java:1402)
    at com.vaadin.server.VaadinServlet.service(VaadinServlet.java:305)
    at javax.servlet.http.HttpServlet.service(HttpServlet.java:725)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:291)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206)
    at org.apache.tomcat.websocket.server.WsFilter.doFilter(WsFilter.java:52)
    at org.apache.catalina.core.ApplicationFilterChain.internalDoFilter(ApplicationFilterChain.java:239)
    at org.apache.catalina.core.ApplicationFilterChain.doFilter(ApplicationFilterChain.java:206)
    at org.apache.catalina.core.StandardWrapperValve.invoke(StandardWrapperValve.java:219)
    at org.apache.catalina.core.StandardContextValve.invoke(StandardContextValve.java:106)
    at org.apache.catalina.authenticator.AuthenticatorBase.invoke(AuthenticatorBase.java:503)
    at org.apache.catalina.core.StandardHostValve.invoke(StandardHostValve.java:136)
    at org.apache.catalina.valves.ErrorReportValve.invoke(ErrorReportValve.java:79)
    at org.apache.catalina.valves.AbstractAccessLogValve.invoke(AbstractAccessLogValve.java:610)
    at org.apache.catalina.core.StandardEngineValve.invoke(StandardEngineValve.java:88)
    at org.apache.catalina.connector.CoyoteAdapter.service(CoyoteAdapter.java:526)
    at org.apache.coyote.http11.AbstractHttp11Processor.process(AbstractHttp11Processor.java:1078)
    at org.apache.coyote.AbstractProtocol$AbstractConnectionHandler.process(AbstractProtocol.java:655)
    at org.apache.coyote.http11.Http11NioProtocol$Http11ConnectionHandler.process(Http11NioProtocol.java:222)
    at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1566)
    at org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.run(NioEndpoint.java:1523)
    at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
    at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
    at org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
    at java.lang.Thread.run(Thread.java:745)
evenbits commented 9 years ago

Maybe instead of having a listener per node it would be more convenient to have one listener per graph and figure out what to do by the nodeId inside event?

sameeraroshan commented 9 years ago

listener is indeed for graph. you can add nodes to the graph when the listener fires. may be you are trying to remove while iterating. for that copy objects to separate list and remove after the iteration. can you send me the code of that section.

On Tue, Mar 31, 2015 at 12:40 AM, Hans Peter Gundelwein < notifications@github.com> wrote:

Maybe instead of having a listener per node it would be more convenient to have one listener per graph and figure out what to do by the nodeId inside event?

— Reply to this email directly or view it on GitHub https://github.com/sameeraroshan/visjs/issues/7#issuecomment-87794917.

evenbits commented 9 years ago

No, I don't remove any nodes. I think the following code should make this more transparent.

    public void setActiveData(ViewObject viewObject) {

        if(viewObject != null) {
            graphBuilder=new GraphBuilder(viewObject);
            graphBuilder.build();

            getNetworkGraph().clear();
            getNetworkGraph().addNodes(graphBuilder.getNodes());
            getNetworkGraph().addEdges(graphBuilder.getEdges());
            registerListener(graphBuilder.getNodes());
        } else {
            getNetworkGraph().clear();
        }
    }

    private void registerListener(List<Node> nodes) {
        for(Node node : nodes) {
            getNetworkGraph().addNodeSelectListener(new Node.NodeSelectListener(node) {
                @Override
                public void onFired(SelectEvent selectEvent) {
                    onSelect(selectEvent.getNodeIds());
                }
            });
        }
    }

    private void onSelect(List<String> nodesId) {
        for(String idStr : nodesId) {
            ViewObject viewObject=findData(idStr);

            if(isExpandable(viewObject)) {
                graphBuilder.extend( viewObject);

                for(Node extNode : graphBuilder.getExtendedNodes()) {
                    getNetworkGraph().addNode(extNode);
                }
                for(Edge extEdge : graphBuilder.getExtendedEdges()) {
                    getNetworkGraph().addEdge(extEdge);
                }
                registerListener(graphBuilder.getExtendedNodes());

                Node graphNode=graphBuilder.findNode( node);

                graphNode.setColor(expandedColor);
                getNetworkGraph().updateNode(graphNode);
            }
        }
    }
rafal-software commented 9 years ago

I have the same problem. java.util.ConcurrentModificationException The issue is that I want to adding nodes to graph by double click. Ive own widget for visjs network - I created my widget a few months ago. In my widget this effect its working, but I have some other problems... Additional problem is that I have 3 edges for two nodes but I`m putting into edges collection only one edge for these nodes.

rafal-software commented 9 years ago

I`ve solved the problem in my local repository. Replacing ArrayList by java.util.concurrent.CopyOnWriteArrayList the line below:

private List<Node.NodeDoubleClickListener> nodeDoubleClickListeners = new CopyOnWriteArrayList();

My suggestion is to replace all ArrayLists on this collection.

sameeraroshan commented 9 years ago

could you please provide me a sample code. I cannot recreate this issue.

Swoorup commented 9 years ago

I am encountering the same issue as well.

Swoorup commented 9 years ago

Can confirm that this fixes the issue albeit not sure if it has any drawbacks private List nodeDoubleClickListeners = new CopyOnWriteArrayList<>();