[ 
https://issues.apache.org/jira/browse/ARTEMIS-2095?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16627641#comment-16627641
 ] 

ASF GitHub Bot commented on ARTEMIS-2095:
-----------------------------------------

Github user michaelandrepearce commented on a diff in the pull request:

    https://github.com/apache/activemq-artemis/pull/2328#discussion_r220273130
  
    --- Diff: 
artemis-commons/src/main/java/org/apache/activemq/artemis/utils/collections/TypedProperties.java
 ---
    @@ -68,22 +73,23 @@ public TypedProperties() {
         *  Return the number of properties
         * */
        public int size() {
    -      return properties.size();
    +      return withReadLock(() -> properties.size());
        }
     
        public int getMemoryOffset() {
           // The estimate is basically the encode size + 2 object references 
for each entry in the map
           // Note we don't include the attributes or anything else since they 
already included in the memory estimate
           // of the ServerMessage
     
    -      return properties == null ? 0 : size + 2 * DataConstants.SIZE_INT * 
properties.size();
    +      return properties == null ? 0 : encodeSize + 2 * 
DataConstants.SIZE_INT * size();
        }
     
        public TypedProperties(final TypedProperties other) {
    -      synchronized (other) {
    +      other.withReadLock(() -> {
              properties = other.properties == null ? null : new 
HashMap<>(other.properties);
    -         size = other.size;
    -      }
    +         ENCODE_SIZE_FIELD_UPDATER.set(this, other.encodeSize);
    --- End diff --
    
    Yes this is in constructor, afaik they can only be single thread at this 
point, ill change to lazySet for the constructor one.


> TypedProperties thread safety
> -----------------------------
>
>                 Key: ARTEMIS-2095
>                 URL: https://issues.apache.org/jira/browse/ARTEMIS-2095
>             Project: ActiveMQ Artemis
>          Issue Type: Bug
>            Reporter: Michael Andre Pearce
>            Assignee: Michael Andre Pearce
>            Priority: Major
>
> Whilst TypedProperties is meant to have a single thread acting on it (and 
> this is the most typical interaction), there are occurrences where, it can be 
> acted on by other threads, when this occurs some concurrent modification 
> errors can occur.
> As such TypedProperties must be thread safe, but concurrency tuning should 
> factor in to be most performant for single thread.
>  
> e.g. 
> 2018-09-24 15:01:27,751 WARN 
> [org.apache.activemq.artemis.core.message.impl.CoreMessage] Error creating 
> String for message: : java.util.ConcurrentModificationException at 
> java.util.HashMap$HashIterator.nextNode(HashMap.java:1437) [rt.jar:1.8.0_102] 
> at java.util.HashMap$EntryIterator.next(HashMap.java:1471) [rt.jar:1.8.0_102] 
> at java.util.HashMap$EntryIterator.next(HashMap.java:1469) [rt.jar:1.8.0_102] 
> at 
> org.apache.activemq.artemis.utils.collections.TypedProperties.toString(TypedProperties.java:464)



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to