Skip to content

Commit

Permalink
Make access to ArrayDeque synchronized
Browse files Browse the repository at this point in the history
ArrayDeque specifies that:

> Array deques ... are not thread-safe; in the absence of external
> synchronization, they do not support concurrent access by multiple
> threads.

`marshalerPool` is concurrently added to by the OkHttp threadpool
without synchronization, along with all threads that end spans (with
synchronisation in `SimpleSpanProcessor.exportLock`, which is not used
to synchronize with the OkHttp threadpool).

Just making the ArrayQueue synchronous internally removes all need to
worry about upstream locks.

Fixes #7019
  • Loading branch information
Mahoney committed Jan 16, 2025
1 parent c8da020 commit df49e28
Showing 1 changed file with 20 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
import io.opentelemetry.sdk.trace.data.SpanData;
import java.util.ArrayDeque;
import java.util.Collection;
import java.util.Deque;
import java.util.Queue;
import java.util.function.BiFunction;

/**
Expand All @@ -20,7 +20,8 @@
*/
public class SpanReusableDataMarshaler {

private final Deque<LowAllocationTraceRequestMarshaler> marshalerPool = new ArrayDeque<>();
private final SynchronizedQueue<LowAllocationTraceRequestMarshaler> marshalerPool
= new SynchronizedQueue<>(new ArrayDeque<>());

private final MemoryMode memoryMode;
private final BiFunction<Marshaler, Integer, CompletableResultCode> doExport;
Expand Down Expand Up @@ -55,4 +56,21 @@ public CompletableResultCode export(Collection<SpanData> spans) {
TraceRequestMarshaler request = TraceRequestMarshaler.create(spans);
return doExport.apply(request, spans.size());
}

private static class SynchronizedQueue<T> {
private final Queue<T> queue;

private SynchronizedQueue(Queue<T> queue) {
this.queue = queue;
}

@SuppressWarnings("UnusedReturnValue")
public synchronized boolean add(T t) {
return queue.add(t);
}

public synchronized T poll() {
return queue.poll();
}
}
}

0 comments on commit df49e28

Please sign in to comment.