Remove subsegments lock (#278)

* Remove subsegments lock

* Deprecate getSubsegments

* Return a copy of subsegments list

* json-ignore

Co-authored-by: William Armiros <[email protected]>

Author: Anuraag Agrawal

aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySegment.java

@@ -241,6 +241,11 @@ public List<Subsegment> getSubsegments() {
         return list;
     }
 
+    @Override
+    public List<Subsegment> getSubsegmentsCopy() {
+        return new ArrayList<>(list);
+    }
+
     @Override
     public void addSubsegment(Subsegment subsegment) {
     }

aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/DummySubsegment.java

@@ -223,6 +223,11 @@ public List<Subsegment> getSubsegments() {
         return list;
     }
 
+    @Override
+    public List<Subsegment> getSubsegmentsCopy() {
+        return new ArrayList<>(list);
+    }
+
     @Override
     public void addSubsegment(Subsegment subsegment) {
     }

aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/Entity.java

@@ -158,7 +158,7 @@ default void run(Runnable runnable, AWSXRayRecorder recorder) {
     void setNamespace(String namespace);
 
     /**
-     * @return the subsegmentsLock
+     * @return an unused {@link ReentrantLock}
      *
      * @deprecated This is for internal use of the SDK and will be made private.
      */
@@ -367,9 +367,18 @@ default void run(Runnable runnable, AWSXRayRecorder recorder) {
 
     /**
      * @return the subsegments
+     *
+     * @deprecated Use {@link #getSubsegmentsCopy()}.
      */
+    @Deprecated
     List<Subsegment> getSubsegments();
 
+    /**
+     * Returns a copy of the currently added subsegments. Updates to the returned {@link List} will not be reflected in the
+     * {@link Entity}.
+     */
+    List<Subsegment> getSubsegmentsCopy();
+
     /**
      * Adds a subsegment.
      *

aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/EntityImpl.java

@@ -530,16 +530,19 @@ public List<Subsegment> getSubsegments() {
         }
     }
 
+    @JsonIgnore
+    @Override
+    public List<Subsegment> getSubsegmentsCopy() {
+        synchronized (lock) {
+            return new ArrayList<>(subsegments);
+        }
+    }
+
     @Override
     public void addSubsegment(Subsegment subsegment) {
         synchronized (lock) {
             checkAlreadyEmitted();
-            getSubsegmentsLock().lock();
-            try {
-                subsegments.add(subsegment);
-            } finally {
-                getSubsegmentsLock().unlock();
-            }
+            subsegments.add(subsegment);
         }
     }
 
@@ -548,13 +551,8 @@ public void addException(Throwable exception) {
         synchronized (lock) {
             checkAlreadyEmitted();
             setFault(true);
-            getSubsegmentsLock().lock();
-            try {
-                cause.addExceptions(creator.getThrowableSerializationStrategy()
-                                           .describeInContext(this, exception, subsegments));
-            } finally {
-                getSubsegmentsLock().unlock();
-            }
+            cause.addExceptions(creator.getThrowableSerializationStrategy()
+                                       .describeInContext(this, exception, subsegments));
         }
     }
 
@@ -756,12 +754,7 @@ public String prettySerialize() {
     @Override
     public void removeSubsegment(Subsegment subsegment) {
         synchronized (lock) {
-            getSubsegmentsLock().lock();
-            try {
-                subsegments.remove(subsegment);
-            } finally {
-                getSubsegmentsLock().unlock();
-            }
+            subsegments.remove(subsegment);
             getParentSegment().getTotalSize().decrement();
         }
     }

aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/NoOpSegment.java

@@ -282,6 +282,11 @@ public List<Subsegment> getSubsegments() {
         return NoOpList.get();
     }
 
+    @Override
+    public List<Subsegment> getSubsegmentsCopy() {
+        return NoOpList.get();
+    }
+
     @Override
     public void addSubsegment(Subsegment subsegment) {
     }

aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/entities/NoOpSubSegment.java

@@ -225,6 +225,11 @@ public List<Subsegment> getSubsegments() {
         return NoOpList.get();
     }
 
+    @Override
+    public List<Subsegment> getSubsegmentsCopy() {
+        return NoOpList.get();
+    }
+
     @Override
     public void addSubsegment(Subsegment subsegment) {
     }

aws-xray-recorder-sdk-core/src/main/java/com/amazonaws/xray/strategy/DefaultStreamingStrategy.java

@@ -20,6 +20,7 @@
 import com.amazonaws.xray.entities.Segment;
 import com.amazonaws.xray.entities.Subsegment;
 import java.util.ArrayList;
+import java.util.List;
 
 public class DefaultStreamingStrategy implements StreamingStrategy {
 
@@ -82,30 +83,18 @@ public boolean requiresStreaming(Segment segment) {
      */
     @Override
     public void streamSome(Entity entity, Emitter emitter) {
-        if (entity.getSubsegmentsLock().tryLock()) {
-            try {
-                stream(entity, emitter);
-            } finally {
-                entity.getSubsegmentsLock().unlock();
-            }
-        }
+        stream(entity, emitter);
     }
 
     private boolean stream(Entity entity, Emitter emitter) {
-        ArrayList<Subsegment> children = new ArrayList<>(entity.getSubsegments());
-        ArrayList<Subsegment> streamable = new ArrayList<>();
+        List<Subsegment> children = entity.getSubsegmentsCopy();
+        List<Subsegment> streamable = new ArrayList<>();
 
         //Gather children and in the condition they are ready to stream, add them to the streamable list.
         if (children.size() > 0) {
             for (Subsegment child : children) {
-                if (child.getSubsegmentsLock().tryLock()) {
-                    try {
-                        if (stream(child, emitter)) {
-                            streamable.add(child);
-                        }
-                    } finally {
-                        child.getSubsegmentsLock().unlock();
-                    }
+                if (stream(child, emitter)) {
+                    streamable.add(child);
                 }
             }
         }