Skip to content

Commit

Permalink
fix: intermitent error on multithread (#70)
Browse files Browse the repository at this point in the history
  • Loading branch information
Orlando Krause committed Jul 17, 2024
1 parent c79e21a commit c580177
Show file tree
Hide file tree
Showing 3 changed files with 51 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,21 @@
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.BiConsumer;

public class ScopedContext implements AutoCloseable {
static final ThreadLocal<Context> context = ThreadLocal.withInitial(() ->
new Context(0L, Collections.emptyMap())
);
static final Map<Thread, Context> threadContext = new ConcurrentHashMap<>();

final Context previous;
final Context current;
final Thread originalThread;
final Ref<Map<Ref<String>, Ref<String>>> currentRef;
boolean closed = false;

public ScopedContext(LabelsSet labels) {
previous = context.get();
originalThread = Thread.currentThread();
previous = getContext();
Map<Ref<String>, Ref<String>> nextContext = new HashMap<>(
previous.labels.size() + labels.args.length / 2
);
Expand Down Expand Up @@ -58,7 +60,7 @@ public ScopedContext(LabelsSet labels) {

AsyncProfiler.getInstance().setContextId(currentRef.id);
current = new Context(currentRef.id, nextContext);
context.set(current);
setContext(current);
}


Expand All @@ -69,7 +71,7 @@ public void close() {
}
closed = true;
currentRef.refCount.decrementAndGet();
context.set(previous);
setContext(previous);
AsyncProfiler.getInstance().setContextId(previous.id);
}

Expand All @@ -79,6 +81,18 @@ public void forEach(BiConsumer<String, String> consumer) {
}
}

private Context getContext() {
return threadContext.computeIfAbsent(this.originalThread, k -> new Context(0L, Collections.emptyMap()));
}

private void setContext(Context context) {
threadContext.put(this.originalThread, context);
}

static Context currentContext() {
return threadContext.get(Thread.currentThread());
}

static class Context {
public final Long id;
public final Map<Ref<String>, Ref<String>> labels;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ void testOneLabelSet() {
assertEquals(1, v1.refCount.get());
}
}
assertEquals(0, ScopedContext.context.get().id);
assertEquals(0, ScopedContext.currentContext().id);
assertEquals(0, ctxRef.refCount.get());
assertEquals(1, k1.refCount.get());
assertEquals(1, v1.refCount.get());
Expand Down Expand Up @@ -195,7 +195,7 @@ void testLabelsSetMerge() {

}
Pyroscope.LabelsWrapper.dump();
assertEquals(0, ScopedContext.context.get().id);
assertEquals(0, ScopedContext.currentContext().id);
assertEquals(0, RefCounted.strings.valueToRef.size());
assertEquals(0, RefCounted.contexts.valueToRef.size());
}
Expand Down Expand Up @@ -226,7 +226,7 @@ void stressTest() throws InterruptedException {
e.shutdown();
e.awaitTermination(100, TimeUnit.SECONDS);
Snapshot res = Pyroscope.LabelsWrapper.dump();
assertEquals(0, ScopedContext.context.get().id);
assertEquals(0, ScopedContext.currentContext().id);
assertEquals(0, RefCounted.strings.valueToRef.size());
assertEquals(0, RefCounted.contexts.valueToRef.size());
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
package io.pyroscope.labels;

import io.pyroscope.labels.io.pyroscope.PyroscopeAsyncProfiler;
import org.junit.jupiter.api.Test;

import java.util.concurrent.ExecutionException;
import java.util.concurrent.ExecutorService;
import java.util.concurrent.Executors;

public class ScopedContextTest {

static {
PyroscopeAsyncProfiler.getAsyncProfiler();
}


/**
* This test demonstrates the issue reported <a href="https://github.com/grafana/pyroscope-java/issues/70">here</a>
*/
@Test
void closeContextOnOtherThread() throws ExecutionException, InterruptedException {
ExecutorService executorService = Executors.newFixedThreadPool(1);
ScopedContext context = new ScopedContext(new LabelsSet("k1", "v1"));
executorService.submit(context::close).get();
Pyroscope.LabelsWrapper.dump();
new ScopedContext(new LabelsSet("k1", "v1"));
}
}

0 comments on commit c580177

Please sign in to comment.