diff --git a/CHANGELOG.md b/CHANGELOG.md index 440881b594..1dc5db2f5b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ ### Fixes - Add support for setting in-app-includes/in-app-excludes via AndroidManifest.xml ([#4240](https://github.com/getsentry/sentry-java/pull/4240)) +- Avoid unnecessary copies and ensure thread-safety when using `CopyOnWriteArrayList` ([#4247](https://github.com/getsentry/sentry-java/pull/4247)) + - This affects in particular `SentryTracer.getLatestActiveSpan` which would have previously unnecessarily copied all children spans, with high impact on memory usage ### Features diff --git a/sentry/src/main/java/io/sentry/Scope.java b/sentry/src/main/java/io/sentry/Scope.java index 4dacc8e4da..d8eecb4708 100644 --- a/sentry/src/main/java/io/sentry/Scope.java +++ b/sentry/src/main/java/io/sentry/Scope.java @@ -759,7 +759,7 @@ public void removeContexts(final @NotNull String key) { @NotNull @Override public List getAttachments() { - return new CopyOnWriteArrayList<>(attachments); + return attachments; } /** diff --git a/sentry/src/main/java/io/sentry/SentryTracer.java b/sentry/src/main/java/io/sentry/SentryTracer.java index 36329db7a7..7e9de25fbf 100644 --- a/sentry/src/main/java/io/sentry/SentryTracer.java +++ b/sentry/src/main/java/io/sentry/SentryTracer.java @@ -7,7 +7,6 @@ import io.sentry.util.AutoClosableReentrantLock; import io.sentry.util.Objects; import io.sentry.util.SpanUtils; -import java.util.ArrayList; import java.util.List; import java.util.ListIterator; import java.util.Map; @@ -155,7 +154,10 @@ private void onDeadlineTimeoutReached() { // abort all child-spans first, this ensures the transaction can be finished, // even if waitForChildren is true // iterate in reverse order to ensure leaf spans are processed before their parents - @NotNull final ListIterator iterator = children.listIterator(children.size()); + @NotNull final ListIterator iterator = this.children.listIterator(); + while (iterator.hasNext()) { + iterator.next(); + } while (iterator.hasPrevious()) { @NotNull final Span span = iterator.previous(); span.setSpanFinishedCallback(null); @@ -677,14 +679,13 @@ private void updateBaggageValues(final @NotNull Baggage baggage) { } private boolean hasAllChildrenFinished() { - final List spans = new ArrayList<>(this.children); - if (!spans.isEmpty()) { - for (final Span span : spans) { - // This is used in the spanFinishCallback, when the span isn't finished, but has a finish - // date - if (!span.isFinished() && span.getFinishDate() == null) { - return false; - } + @NotNull final ListIterator iterator = this.children.listIterator(); + while (iterator.hasNext()) { + @NotNull final Span span = iterator.next(); + // This is used in the spanFinishCallback, when the span isn't finished, but has a finish + // date + if (!span.isFinished() && span.getFinishDate() == null) { + return false; } } return true; @@ -909,12 +910,14 @@ public void setName(@NotNull String name, @NotNull TransactionNameSource transac @Override public @Nullable ISpan getLatestActiveSpan() { - final List spans = new ArrayList<>(this.children); - if (!spans.isEmpty()) { - for (int i = spans.size() - 1; i >= 0; i--) { - if (!spans.get(i).isFinished()) { - return spans.get(i); - } + @NotNull final ListIterator iterator = this.children.listIterator(); + while (iterator.hasNext()) { + iterator.next(); + } + while (iterator.hasPrevious()) { + @NotNull final Span span = iterator.previous(); + if (!span.isFinished()) { + return span; } } return null;