From dc9186b31c36fbb1025f7daf913bfc1b80a918a1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Basl=C3=A9?= Date: Mon, 29 Mar 2021 11:05:32 +0200 Subject: [PATCH] Use a BuilderAllowSpec, as a phase of the builder --- .../java/reactor/blockhound/BlockHound.java | 85 +++++++++++-- .../com/example/BlockingAllowSpecTest.java | 113 ++++++++++++++++++ .../test/java/com/example/StaticInitTest.java | 5 +- 3 files changed, 187 insertions(+), 16 deletions(-) create mode 100644 example/src/test/java/com/example/BlockingAllowSpecTest.java diff --git a/agent/src/main/java/reactor/blockhound/BlockHound.java b/agent/src/main/java/reactor/blockhound/BlockHound.java index f0fc177..834b9f8 100644 --- a/agent/src/main/java/reactor/blockhound/BlockHound.java +++ b/agent/src/main/java/reactor/blockhound/BlockHound.java @@ -62,12 +62,6 @@ */ public class BlockHound { - /** - * The special methodName that should be used in e.g. {@link Builder#allowBlockingCallsInside(String, String)} - * when one wants to reference the static initializer block rather than a class method. - */ - public static final String STATIC_INITIALIZER = ""; - static final String PREFIX = "$$BlockHound$$_"; private static final AtomicBoolean INITIALIZED = new AtomicBoolean(false); @@ -126,6 +120,10 @@ public TypePool typePool(ClassFileLocator classFileLocator, ClassLoader classLoa } } + /** + * A builder for configuration of the BlockHound java agent, passed to each {@link BlockHoundIntegration}'s + * {@link BlockHoundIntegration#applyTo(Builder) applyTo} method when {@link BlockHound#install(BlockHoundIntegration...) installing}. + */ public static class Builder { private final Map>> blockingMethods = new HashMap>>() {{ @@ -260,6 +258,58 @@ public static class Builder { private Predicate dynamicThreadPredicate = t -> false; + /** + * A phase of the builder where allowed blocking methods are configured for a given class. + * See {@link Builder#allowBlockingCallsInside(String)}. + *

+ * Note that methods of this class directly mutate the {@link Builder} as if one was calling + * {@link Builder#allowBlockingCallsInside(String, String)}. One MUST specify at least one + * allowed method for it to have an effect on the configuration. + */ + public class BuilderAllowSpec { + + final String className; + + private BuilderAllowSpec(String className) { + this.className = className; + } + + /** + * Allow blocking calls inside the static initializer block of the currently configured + * class. Use {@link #and()} to exit the class-specific phase and return + * to the {@link Builder}. + * + * @return this {@link BuilderAllowSpec} for further configuration of the given class + * @see #and() + */ + public BuilderAllowSpec forStaticInitializer() { + Builder.this.allowBlockingCallsInside(this.className, ""); + return this; + } + + /** + * Allow blocking calls inside the specified method of the currently configured + * class. Use {@link #and()} to exit the class-specific phase and return + * to the {@link Builder}. + * + * @return this {@link BuilderAllowSpec} for further configuration of the given class + * @see #and() + */ + public BuilderAllowSpec forMethod(String methodName) { + Builder.this.allowBlockingCallsInside(this.className, methodName); + return this; + } + + /** + * Exit the class-specific phase and return to the {@link Builder}. + * + * @return the {@link Builder} + */ + public Builder and() { + return Builder.this; + } + } + /** * Marks provided method of the provided class as "blocking". * @@ -297,30 +347,39 @@ public Builder markAsBlocking(String className, String methodName, String signat * Allows blocking calls inside any method of a class with name identified by the provided className * and which name matches the provided methodName. *

- * Note that for static initializers, you can use {@link BlockHound#STATIC_INITIALIZER} as the methodName. - * Constructors are currently not supported. + * A sub-builder is available to guide users towards more advanced cases like static initializers, + * see {@link #allowBlockingCallsInside(String)} and {@link BuilderAllowSpec}. * * @param className class' name (e.g. "java.lang.Thread") * @param methodName a method name * @return this - * @see BlockHound#STATIC_INITIALIZER + * @see #allowBlockingCallsInside(String) + * @see BuilderAllowSpec */ public Builder allowBlockingCallsInside(String className, String methodName) { allowances.computeIfAbsent(className, __ -> new HashMap<>()).put(methodName, true); return this; } + /** + * Configure allowed blocking calls inside methods of a class with name identified by the provided className. + * Note that this doesn't change the configuration unless one further specifies which methods are allowed. + * Use {@link BuilderAllowSpec#and()} to get back to this {@link Builder}. + * + * @param className class' name (e.g. "java.lang.Thread") + * @return a sub-step of the builder as a {@link BuilderAllowSpec} + */ + public BuilderAllowSpec allowBlockingCallsInside(String className) { + return new BuilderAllowSpec(className); + } + /** * Disallows blocking calls inside any method of a class with name identified by the provided className * and which name matches the provided methodName. - *

- * Note that for static initializers, you can use {@link BlockHound#STATIC_INITIALIZER} as the methodName. - * Constructors are currently not supported. * * @param className class' name (e.g. "java.lang.Thread") * @param methodName a method name * @return this - * @see BlockHound#STATIC_INITIALIZER */ public Builder disallowBlockingCallsInside(String className, String methodName) { allowances.computeIfAbsent(className, __ -> new HashMap<>()).put(methodName, false); diff --git a/example/src/test/java/com/example/BlockingAllowSpecTest.java b/example/src/test/java/com/example/BlockingAllowSpecTest.java new file mode 100644 index 0000000..a1b588c --- /dev/null +++ b/example/src/test/java/com/example/BlockingAllowSpecTest.java @@ -0,0 +1,113 @@ +/* + * Copyright (c) 2020-Present Pivotal Software Inc, All Rights Reserved. + * + * 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 + * + * https://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.example; + +import org.assertj.core.api.Assertions; +import org.junit.Test; + +import reactor.blockhound.BlockHound; +import reactor.blockhound.BlockingOperationError; +import reactor.core.publisher.Mono; +import reactor.core.scheduler.Schedulers; + +public class BlockingAllowSpecTest { + + static { + BlockHound.install(b -> b + .allowBlockingCallsInside(BlockingClassA.class.getName()) + .forStaticInitializer() + .forMethod("block") + .and() + .allowBlockingCallsInside(BlockingClassB.class.getName()) + .forMethod("block1") + .forMethod("block2") + ); + } + + @Test + public void shouldInstrumentBlockingClassA() { + Mono.fromCallable(BlockingClassA::new) + .map(BlockingClassA::block) + .subscribeOn(Schedulers.parallel()) + .block(); + } + + @Test + public void shouldInstrumentBlockingClassB() { + Mono.fromCallable(() -> { + BlockingClassB b = new BlockingClassB(); + b.block1(); + b.block2(); + return b; + }).subscribeOn(Schedulers.parallel()).block(); + } + + @Test + public void usingAllowSpecWithoutCallingMethodIsIgnored() { + //getting the BuilderAllowSpec and not acting on it DOESN'T equate to allowing any method: it does nothing + BlockHound.install(b -> b.allowBlockingCallsInside(BlockingClassC.class.getName())); + + Mono mono = Mono.fromCallable(BlockingClassC::new) + .publishOn(Schedulers.parallel()) + .map(BlockingClassC::block1); + + Assertions.assertThatExceptionOfType(RuntimeException.class) + .isThrownBy(mono::block) + .havingCause() + .isInstanceOf(BlockingOperationError.class) + .withMessage("Blocking call! java.lang.Thread.yield") + .withStackTraceContaining("at com.example.BlockingAllowSpecTest$BlockingClassC.block1"); + } + + static class BlockingClassA { + static { + try { + Thread.sleep(0); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + } + + String block() { + Thread.yield(); + return "hello A"; + } + } + + static class BlockingClassB { + + void block1() { + try { + Thread.sleep(0); + } catch (InterruptedException e) { + throw new RuntimeException(e); + } + } + + void block2() { + Thread.yield(); + } + } + + static class BlockingClassC { + + String block1() { + Thread.yield(); + return "hello C"; + } + } +} diff --git a/example/src/test/java/com/example/StaticInitTest.java b/example/src/test/java/com/example/StaticInitTest.java index 7584c65..f69006e 100644 --- a/example/src/test/java/com/example/StaticInitTest.java +++ b/example/src/test/java/com/example/StaticInitTest.java @@ -17,6 +17,7 @@ package com.example; import org.junit.Test; + import reactor.blockhound.BlockHound; import reactor.core.publisher.Mono; import reactor.core.scheduler.Schedulers; @@ -24,9 +25,7 @@ public class StaticInitTest { static { - BlockHound.install(b -> { - b.allowBlockingCallsInside(ClassWithStaticInit.class.getName(), BlockHound.STATIC_INITIALIZER); - }); + BlockHound.install(b -> b.allowBlockingCallsInside(ClassWithStaticInit.class.getName()).forStaticInitializer()); } @Test