You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@plc4x.apache.org by hu...@apache.org on 2023/02/13 11:02:42 UTC

[plc4x] branch develop updated: Fix code gen concurrent modification (#795)

This is an automated email from the ASF dual-hosted git repository.

hutcheb pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/plc4x.git


The following commit(s) were added to refs/heads/develop by this push:
     new 3f13c0baf9 Fix code gen concurrent modification (#795)
3f13c0baf9 is described below

commit 3f13c0baf967fe234578b9f4c25c3c03cfa015f6
Author: Ben Hutcheson <be...@gmail.com>
AuthorDate: Mon Feb 13 12:02:35 2023 +0100

    Fix code gen concurrent modification (#795)
    
    * fix(plc4j/code-gen): fix for concurrent modification when adding multiple mspecs to context.
    
    * fix(plc4j/code-gen): remove TODO and add ads discovery mspec to validation checks
    
    * fix(plc4j/code-gen): revert ads protocol type context change
    
    * fix(code-gen): Use CopyonWrite Array instead of linked list. Bit of a workaround
---
 .../language/mspec/parser/MessageFormatListener.java    | 17 ++++++++++-------
 .../plc4x/protocol/bacnetip/BacNetIpProtocol.java       |  1 -
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/code-generation/protocol-base-mspec/src/main/java/org/apache/plc4x/plugins/codegenerator/language/mspec/parser/MessageFormatListener.java b/code-generation/protocol-base-mspec/src/main/java/org/apache/plc4x/plugins/codegenerator/language/mspec/parser/MessageFormatListener.java
index 4f58055b5d..293a6ef3fb 100644
--- a/code-generation/protocol-base-mspec/src/main/java/org/apache/plc4x/plugins/codegenerator/language/mspec/parser/MessageFormatListener.java
+++ b/code-generation/protocol-base-mspec/src/main/java/org/apache/plc4x/plugins/codegenerator/language/mspec/parser/MessageFormatListener.java
@@ -49,6 +49,7 @@ import java.nio.charset.Charset;
 import java.util.*;
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.CompletionStage;
+import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.function.Consumer;
 import java.util.stream.Collectors;
 
@@ -880,16 +881,18 @@ public class MessageFormatListener extends MSpecBaseListener implements LazyType
 
         types.put(typeName, type);
 
-        List<Consumer<TypeDefinition>> waitingConsumers = typeDefinitionConsumers.getOrDefault(typeName, new LinkedList<>());
+        // TODO:- Figure out why we need a write on copy array to get around a Concurrent Modification Exception being raised.
+        List<Consumer<TypeDefinition>> waitingConsumers = typeDefinitionConsumers.getOrDefault(typeName, new CopyOnWriteArrayList<>());
         LOGGER.debug("{} waiting for {}", waitingConsumers.size(), typeName);
 
-        Iterator<Consumer<TypeDefinition>> consumerIterator = waitingConsumers.iterator();
-        while (consumerIterator.hasNext()) {
-            Consumer<TypeDefinition> setter = consumerIterator.next();
+        LinkedList<Consumer<TypeDefinition>> removeList = new LinkedList<>();
+        for (Consumer<TypeDefinition> setter : waitingConsumers) {
             LOGGER.debug("setting {} for {}", typeName, setter);
             setter.accept(type);
-            consumerIterator.remove();
+            removeList.add(setter);
         }
+
+        waitingConsumers.removeAll(removeList);
         typeDefinitionConsumers.remove(typeName);
     }
 
@@ -904,9 +907,9 @@ public class MessageFormatListener extends MSpecBaseListener implements LazyType
         } else {
             // put up order
             if (LOGGER.isDebugEnabled()) {
-                LOGGER.debug("{} already waiting for {}", typeDefinitionConsumers.getOrDefault(typeRefName, new LinkedList<>()).size(), typeRefName);
+                LOGGER.debug("{} already waiting for {}", typeDefinitionConsumers.getOrDefault(typeRefName, new CopyOnWriteArrayList<>()).size(), typeRefName);
             }
-            typeDefinitionConsumers.putIfAbsent(typeRefName, new LinkedList<>());
+            typeDefinitionConsumers.putIfAbsent(typeRefName, new CopyOnWriteArrayList<>());
             typeDefinitionConsumers.get(typeRefName).add(setTypeDefinition);
         }
     }
diff --git a/protocols/bacnetip/src/main/java/org/apache/plc4x/protocol/bacnetip/BacNetIpProtocol.java b/protocols/bacnetip/src/main/java/org/apache/plc4x/protocol/bacnetip/BacNetIpProtocol.java
index 3d9f5ce57c..deeab1e2ba 100644
--- a/protocols/bacnetip/src/main/java/org/apache/plc4x/protocol/bacnetip/BacNetIpProtocol.java
+++ b/protocols/bacnetip/src/main/java/org/apache/plc4x/protocol/bacnetip/BacNetIpProtocol.java
@@ -55,7 +55,6 @@ public class BacNetIpProtocol implements Protocol, ProtocolHelpers {
         LOGGER.info("Parsing: bacnetip.mspec");
         typeContext = new MessageFormatParser().parse(getMspecStream(), typeContext);
 
-        // TODO: those should work above bacnetip.mspec but somehow if we move them we get a concurrent modification exception... debug that.
         LOGGER.info("Parsing: bacnet-vendorids.mspec");
         typeContext = new MessageFormatParser().parse(getMspecStream("bacnet-vendorids"), typeContext);