Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix NoSuchElementException in CircularFifoQueue when cloning a Scope #2328

Merged
merged 3 commits into from Nov 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -5,6 +5,7 @@
### Fixes

- Use correct set-cookie for the HTTP Client response object ([#2326](https://github.com/getsentry/sentry-java/pull/2326))
- Fix NoSuchElementException in CircularFifoQueue when cloning a Scope ([#2328](https://github.com/getsentry/sentry-java/pull/2328))

### Features

Expand Down
2 changes: 1 addition & 1 deletion sentry/src/main/java/io/sentry/Scope.java
Expand Up @@ -97,7 +97,7 @@ public Scope(final @NotNull SentryOptions options) {
this.fingerprint = new ArrayList<>(scope.fingerprint);
this.eventProcessors = new CopyOnWriteArrayList<>(scope.eventProcessors);

final Queue<Breadcrumb> breadcrumbsRef = scope.breadcrumbs;
final Breadcrumb[] breadcrumbsRef = scope.breadcrumbs.toArray(new Breadcrumb[0]);

Queue<Breadcrumb> breadcrumbsClone = createBreadcrumbsList(scope.options.getMaxBreadcrumbs());

Expand Down
14 changes: 14 additions & 0 deletions sentry/src/main/java/io/sentry/SynchronizedQueue.java
Expand Up @@ -131,4 +131,18 @@ public E remove() {
return decorated().remove();
}
}

@Override
public Object[] toArray() {
synchronized (lock) {
return decorated().toArray();
}
}

@Override
public <T> T[] toArray(T[] object) {
synchronized (lock) {
return decorated().toArray(object);
}
}
}
25 changes: 25 additions & 0 deletions sentry/src/test/java/io/sentry/ScopeTest.kt
Expand Up @@ -217,6 +217,31 @@ class ScopeTest {
assertTrue(clone.attachments is CopyOnWriteArrayList)
}

@Test
fun `copying scope won't crash if there are concurrent operations`() {
val options = SentryOptions().apply {
maxBreadcrumbs = 10000
}
val scope = Scope(options)
for (i in 0 until options.maxBreadcrumbs) {
scope.addBreadcrumb(Breadcrumb.info("item"))
}

// remove one breadcrumb after the other on an extra thread
Thread({
while (scope.breadcrumbs.isNotEmpty()) {
scope.breadcrumbs.remove()
}
}, "thread-breadcrumb-remover").start()

// clone in the meantime
while (scope.breadcrumbs.isNotEmpty()) {
Scope(scope)
}

// expect no exception to be thrown ¯\_(ツ)_/¯
}

@Test
fun `clear scope resets scope to default state`() {
val scope = Scope(SentryOptions())
Expand Down