Skip to content

Commit

Permalink
Add detection of ThreadLocal.initialValue() calls
Browse files Browse the repository at this point in the history
  • Loading branch information
spkrka committed May 2, 2022
1 parent 83dea83 commit d38463a
Show file tree
Hide file tree
Showing 5 changed files with 163 additions and 0 deletions.
23 changes: 23 additions & 0 deletions feline/src/main/java/com/spotify/feline/Feline.java
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,24 @@ public static void allowBlockingCallsInside(final String className, final String
});
}

/**
* The consumer will be called every time a ThreadLocal object triggers initialValue(). This
* should be a rare event for well behaving usages of ThreadLocal.
*
* <p>However, since Java 17+, the combination of ThreadLocal and ForkJoinPool.commonPool() may
* lead to overly frequent calls to initialValue() which can be harmful for performance and/or
* correctness. This can be used to detect suspicious high rate of calls.
*
* @param consumer consumer to be invoked on each call to ThreadLocal.initialValue()
*/
public static void addThreadLocalInitialValueConsumer(final Runnable consumer) {
FelineRuntime.addThreadLocalInitialValueConsumer(consumer);
}

public static boolean removeThreadLocalInitialValueConsumer(final Runnable consumer) {
return FelineRuntime.removeThreadLocalInitialValueConsumer(consumer);
}

static {
final Instrumentation instrumentation = ByteBuddyAgent.install();

Expand Down Expand Up @@ -198,6 +216,11 @@ public static void allowBlockingCallsInside(final String className, final String
.type(it -> allowances.containsKey(it.getName()))
.transform(new AllowancesTransformer(allowances))
.asTerminalTransformation()

// instrument ThreadLocal
.type(ElementMatchers.isSubTypeOf(ThreadLocal.class))
.transform(FelineThreadLocalTransformer.forThreadLocal())
.asTerminalTransformation()
.installOn(instrumentation);
}
}
24 changes: 24 additions & 0 deletions feline/src/main/java/com/spotify/feline/FelineRuntime.java
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ public class FelineRuntime {
private static final List<Consumer<Map<String, Object>>> onExitConsumers =
new CopyOnWriteArrayList<>();

private static final List<Runnable> threadLocalInitialValueConsumers =
new CopyOnWriteArrayList<>();

public static void addOnExitConsumerFirst(
final Consumer<Map<String, Object>> blockingCallConsumer) {
onExitConsumers.add(0, blockingCallConsumer);
Expand Down Expand Up @@ -72,4 +75,25 @@ public static void acceptOnExit(final Map<String, Object> data) {
consumer.accept(data);
}
}

public static void addThreadLocalInitialValueConsumer(final Runnable consumer) {
threadLocalInitialValueConsumers.add(consumer);
}

public static boolean removeThreadLocalInitialValueConsumer(final Runnable consumer) {
return threadLocalInitialValueConsumers.remove(consumer);
}

public static void acceptThreadLocalInitialValue() {
// Since this method may be invoked frequently, the regular for-each loop is
// replaced with a manual loop to reduce object creation
final int n = threadLocalInitialValueConsumers.size();
for (int i = 0; i < n; i++) {
try {
threadLocalInitialValueConsumers.get(i).run();
} catch (Exception e) {
// Ignore
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/*
* Copyright (c) 2022 Spotify AB
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not
* use this file except in compliance with the License. You may obtain a copy of
* the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations under
* the License.
*/

package com.spotify.feline;

import static net.bytebuddy.matcher.ElementMatchers.named;

import net.bytebuddy.agent.builder.AgentBuilder;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.description.NamedElement;
import net.bytebuddy.description.type.TypeDescription;
import net.bytebuddy.dynamic.DynamicType;
import net.bytebuddy.matcher.ElementMatcher;
import net.bytebuddy.utility.JavaModule;

class FelineThreadLocalTransformer implements AgentBuilder.Transformer {

private final ElementMatcher.Junction<NamedElement> matcher;

public static AgentBuilder.Transformer forThreadLocal() {
return new FelineThreadLocalTransformer(named("initialValue"));
}

private FelineThreadLocalTransformer(final ElementMatcher.Junction<NamedElement> matcher) {
this.matcher = matcher;
}

@Override
public DynamicType.Builder<?> transform(
final DynamicType.Builder<?> builder,
final TypeDescription typeDescription,
final ClassLoader classLoader,
final JavaModule module) {

return builder.visit(Advice.to(FutureCallAdvice.class).on(matcher));
}

static class FutureCallAdvice {

@Advice.OnMethodEnter
static void onEnter() {
FelineRuntime.acceptThreadLocalInitialValue();
}
}
}
49 changes: 49 additions & 0 deletions feline/src/test/java/com/spotify/feline/ThreadLocalTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/*
* Copyright (c) 2022 Spotify AB
*
* Licensed under the Apache License, Version 2.0 (the "License"); you may not
* use this file except in compliance with the License. You may obtain a copy of
* the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations under
* the License.
*/

package com.spotify.feline;

import static org.junit.jupiter.api.Assertions.assertEquals;

import java.util.concurrent.atomic.AtomicInteger;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;

public class ThreadLocalTest {

private static final AtomicInteger COUNTER = new AtomicInteger();
private static final Runnable RUNNABLE = COUNTER::incrementAndGet;

@BeforeAll
public static void classSetUp() {
Feline.addThreadLocalInitialValueConsumer(RUNNABLE);
}

@AfterAll
public static void teardown() {
Feline.removeThreadLocalInitialValueConsumer(RUNNABLE);
}

@Test
void testThreadLocal() {
final int before = COUNTER.get();
final ThreadLocal<String> threadLocal = ThreadLocal.withInitial(() -> "");
threadLocal.get();
final int after = COUNTER.get();
assertEquals(before + 1, after);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ public class FelineMetricsRecorder {
MetricId.EMPTY.tagged("what", "blocking-calls-time", "unit", "ns");
private final MetricsConsumer.CallFinder callFinder;

private final Meter initialValueCalls;

/**
* Create a MetricsConsumer with the default Predicate. The default Predicate will tag the metric
* with first method name not matching "java." after the blocking call. This will typically give a
Expand All @@ -51,6 +53,8 @@ public FelineMetricsRecorder(
final SemanticMetricRegistry registry, final MetricsConsumer.CallFinder callFinder) {
this.registry = registry;
this.callFinder = callFinder;
this.initialValueCalls =
registry.meter(MetricId.EMPTY.tagged("what", "thread-local-initial-value"));
}

private Optional<StackTraceElement> getBlockingMethod(final String blockingCall) {
Expand Down Expand Up @@ -88,6 +92,11 @@ private static void install(final FelineMetricsRecorder consumer) {
.orElse("unknown");
consumer.markMeter(call, blockedTimeNanos);
});
Feline.addThreadLocalInitialValueConsumer(consumer::acceptThreadLocal);
}

private void acceptThreadLocal() {
initialValueCalls.mark();
}

private void markMeter(final String call, final long timeBlockedNanos) {
Expand Down

0 comments on commit d38463a

Please sign in to comment.