From 525b3b442ca08c56347c8604accb3e74c34fba65 Mon Sep 17 00:00:00 2001 From: Sergei Egorov Date: Thu, 26 Mar 2020 11:15:52 +0100 Subject: [PATCH 1/6] Perform a smoke test after instrumenting the classes. Since OpenJDK requires `-XX:+AllowRedefinitionToAddDeleteMethod` (see #33) starting with Java 13, we should test that the instrumentation was applied correctly and fail fast if not. --- .../java/reactor/blockhound/BlockHound.java | 34 +++++++++++ .../java/reactor/blockhound/TestThread.java | 57 +++++++++++++++++++ 2 files changed, 91 insertions(+) create mode 100644 agent/src/main/java/reactor/blockhound/TestThread.java diff --git a/agent/src/main/java/reactor/blockhound/BlockHound.java b/agent/src/main/java/reactor/blockhound/BlockHound.java index bdde85b..666577e 100644 --- a/agent/src/main/java/reactor/blockhound/BlockHound.java +++ b/agent/src/main/java/reactor/blockhound/BlockHound.java @@ -387,6 +387,7 @@ public void install() { BlockHoundRuntime.dynamicThreadPredicate = dynamicThreadPredicate; // Eagerly trigger the classloading of `threadPredicate` (since classloading is blocking) + threadPredicate = threadPredicate.or(TestThread.class::isInstance); threadPredicate.test(Thread.currentThread()); BlockHoundRuntime.threadPredicate = threadPredicate; @@ -395,6 +396,39 @@ public void install() { catch (Throwable e) { throw new RuntimeException(e); } + + Consumer originalOnBlockingMethod = onBlockingMethod; + onBlockingMethod = m -> { + Thread currentThread = Thread.currentThread(); + if (currentThread instanceof TestThread) { + ((TestThread) currentThread).blockingCallDetected = true; + } + }; + testInstrumentation(); + onBlockingMethod = originalOnBlockingMethod; + } + + private void testInstrumentation() { + TestThread thread = new TestThread(); + thread.startAndWait(); + + // Set in the artificial blockingMethodConsumer, see install() + if (thread.blockingCallDetected) { + return; + } + + String message = "The instrumentation have failed."; + try { + // Test some public API class added in Java 13 + Class.forName("sun.nio.ch.NioSocketImpl"); + message += "\n"; + message += "It looks like you're running on JDK 13+.\n"; + message += "You need to add '-XX:+AllowRedefinitionToAddDeleteMethods' JVM flag.\n"; + message += "See https://github.com/reactor/BlockHound/issues/33 for more info."; + } catch (ClassNotFoundException ignored) { + } + + throw new IllegalStateException(message); } private void instrument(Instrumentation instrumentation) { diff --git a/agent/src/main/java/reactor/blockhound/TestThread.java b/agent/src/main/java/reactor/blockhound/TestThread.java new file mode 100644 index 0000000..bce07b8 --- /dev/null +++ b/agent/src/main/java/reactor/blockhound/TestThread.java @@ -0,0 +1,57 @@ +/* + * 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 reactor.blockhound; + +import java.util.concurrent.FutureTask; +import java.util.concurrent.TimeUnit; + +/** + * This is an internal class for performing the instrumentation test + */ +class TestThread extends Thread { + + volatile boolean blockingCallDetected = false; + + final FutureTask task = new FutureTask<>(() -> { + Thread.sleep(0); + return null; + }); + + TestThread() { + super(); + setName("blockhound-test-thread"); + setDaemon(true); + } + + @Override + public void run() { + task.run(); + } + + public void startAndWait() { + start(); + try { + task.get(5, TimeUnit.SECONDS); + } + catch (Exception e) { + if (e instanceof InterruptedException) { + Thread.currentThread().interrupt(); + } + throw new RuntimeException(e); + } + } +} From b9eb041d48c30b38bb105ece0481d55a6e9bc771 Mon Sep 17 00:00:00 2001 From: Sergei Egorov Date: Thu, 26 Mar 2020 11:35:14 +0100 Subject: [PATCH 2/6] Handle `NoClassDefFoundError` --- agent/src/main/java/reactor/blockhound/BlockHound.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/agent/src/main/java/reactor/blockhound/BlockHound.java b/agent/src/main/java/reactor/blockhound/BlockHound.java index 666577e..0159429 100644 --- a/agent/src/main/java/reactor/blockhound/BlockHound.java +++ b/agent/src/main/java/reactor/blockhound/BlockHound.java @@ -425,7 +425,8 @@ private void testInstrumentation() { message += "It looks like you're running on JDK 13+.\n"; message += "You need to add '-XX:+AllowRedefinitionToAddDeleteMethods' JVM flag.\n"; message += "See https://github.com/reactor/BlockHound/issues/33 for more info."; - } catch (ClassNotFoundException ignored) { + } + catch (ClassNotFoundException | NoClassDefFoundError ignored) { } throw new IllegalStateException(message); From 175b7b014bf68d45e4e99c53f839c5c2a3b04c84 Mon Sep 17 00:00:00 2001 From: Sergei Egorov Date: Thu, 26 Mar 2020 11:45:56 +0100 Subject: [PATCH 3/6] hm... --- agent/src/main/java/reactor/blockhound/BlockHound.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/agent/src/main/java/reactor/blockhound/BlockHound.java b/agent/src/main/java/reactor/blockhound/BlockHound.java index 0159429..39fc99e 100644 --- a/agent/src/main/java/reactor/blockhound/BlockHound.java +++ b/agent/src/main/java/reactor/blockhound/BlockHound.java @@ -426,7 +426,7 @@ private void testInstrumentation() { message += "You need to add '-XX:+AllowRedefinitionToAddDeleteMethods' JVM flag.\n"; message += "See https://github.com/reactor/BlockHound/issues/33 for more info."; } - catch (ClassNotFoundException | NoClassDefFoundError ignored) { + catch (Throwable ignored) { } throw new IllegalStateException(message); From 03a577d29b7196a5dbb77a953a10a25ee981ddf0 Mon Sep 17 00:00:00 2001 From: Sergei Egorov Date: Thu, 26 Mar 2020 11:51:34 +0100 Subject: [PATCH 4/6] debug --- agent/src/main/java/reactor/blockhound/BlockHound.java | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/agent/src/main/java/reactor/blockhound/BlockHound.java b/agent/src/main/java/reactor/blockhound/BlockHound.java index 39fc99e..de83efa 100644 --- a/agent/src/main/java/reactor/blockhound/BlockHound.java +++ b/agent/src/main/java/reactor/blockhound/BlockHound.java @@ -361,6 +361,12 @@ public Builder with(BlockHoundIntegration integration) { * Installs the agent and runs the instrumentation, but only if BlockHound wasn't installed yet (it is global). */ public void install() { + Thread.setDefaultUncaughtExceptionHandler(new Thread.UncaughtExceptionHandler() { + @Override + public void uncaughtException(Thread t, Throwable e) { + e.printStackTrace(); + } + }); try { if (!INITIALIZED.compareAndSet(false, true)) { return; From a0439ea1b59b761bc4acf4ea7caeb4bdf585f0bf Mon Sep 17 00:00:00 2001 From: Sergei Egorov Date: Thu, 26 Mar 2020 12:16:45 +0100 Subject: [PATCH 5/6] override `onBlockingMethod` as early as possible --- .../java/reactor/blockhound/BlockHound.java | 29 ++++++++----------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/agent/src/main/java/reactor/blockhound/BlockHound.java b/agent/src/main/java/reactor/blockhound/BlockHound.java index de83efa..a597526 100644 --- a/agent/src/main/java/reactor/blockhound/BlockHound.java +++ b/agent/src/main/java/reactor/blockhound/BlockHound.java @@ -361,17 +361,19 @@ public Builder with(BlockHoundIntegration integration) { * Installs the agent and runs the instrumentation, but only if BlockHound wasn't installed yet (it is global). */ public void install() { - Thread.setDefaultUncaughtExceptionHandler(new Thread.UncaughtExceptionHandler() { - @Override - public void uncaughtException(Thread t, Throwable e) { - e.printStackTrace(); - } - }); - try { - if (!INITIALIZED.compareAndSet(false, true)) { - return; + if (!INITIALIZED.compareAndSet(false, true)) { + return; + } + + Consumer originalOnBlockingMethod = onBlockingMethod; + onBlockingMethod = m -> { + Thread currentThread = Thread.currentThread(); + if (currentThread instanceof TestThread) { + ((TestThread) currentThread).blockingCallDetected = true; } + }; + try { Instrumentation instrumentation = ByteBuddyAgent.install(); InstrumentationUtils.injectBootstrapClasses( instrumentation, @@ -403,13 +405,6 @@ public void uncaughtException(Thread t, Throwable e) { throw new RuntimeException(e); } - Consumer originalOnBlockingMethod = onBlockingMethod; - onBlockingMethod = m -> { - Thread currentThread = Thread.currentThread(); - if (currentThread instanceof TestThread) { - ((TestThread) currentThread).blockingCallDetected = true; - } - }; testInstrumentation(); onBlockingMethod = originalOnBlockingMethod; } @@ -432,7 +427,7 @@ private void testInstrumentation() { message += "You need to add '-XX:+AllowRedefinitionToAddDeleteMethods' JVM flag.\n"; message += "See https://github.com/reactor/BlockHound/issues/33 for more info."; } - catch (Throwable ignored) { + catch (ClassNotFoundException ignored) { } throw new IllegalStateException(message); From c7d434710b6be43c94acbf599b80da9d34d9aee0 Mon Sep 17 00:00:00 2001 From: Sergei Egorov Date: Thu, 26 Mar 2020 12:45:22 +0100 Subject: [PATCH 6/6] fix the classloading bug --- .../java/reactor/blockhound/BlockHound.java | 32 ++++++++++--------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/agent/src/main/java/reactor/blockhound/BlockHound.java b/agent/src/main/java/reactor/blockhound/BlockHound.java index a597526..d4bb69f 100644 --- a/agent/src/main/java/reactor/blockhound/BlockHound.java +++ b/agent/src/main/java/reactor/blockhound/BlockHound.java @@ -366,13 +366,6 @@ public void install() { } Consumer originalOnBlockingMethod = onBlockingMethod; - onBlockingMethod = m -> { - Thread currentThread = Thread.currentThread(); - if (currentThread instanceof TestThread) { - ((TestThread) currentThread).blockingCallDetected = true; - } - }; - try { Instrumentation instrumentation = ByteBuddyAgent.install(); InstrumentationUtils.injectBootstrapClasses( @@ -390,14 +383,14 @@ public void install() { onBlockingMethod.accept(new BlockingMethod(className, methodName, modifiers)); }; - // Eagerly trigger the classloading of `dynamicThreadPredicate` (since classloading is blocking) - dynamicThreadPredicate.test(Thread.currentThread()); - BlockHoundRuntime.dynamicThreadPredicate = dynamicThreadPredicate; - - // Eagerly trigger the classloading of `threadPredicate` (since classloading is blocking) - threadPredicate = threadPredicate.or(TestThread.class::isInstance); - threadPredicate.test(Thread.currentThread()); - BlockHoundRuntime.threadPredicate = threadPredicate; + onBlockingMethod = m -> { + Thread currentThread = Thread.currentThread(); + if (currentThread instanceof TestThread) { + ((TestThread) currentThread).blockingCallDetected = true; + } + }; + BlockHoundRuntime.dynamicThreadPredicate = t -> false; + BlockHoundRuntime.threadPredicate = TestThread.class::isInstance; instrument(instrumentation); } @@ -406,6 +399,15 @@ public void install() { } testInstrumentation(); + + // Eagerly trigger the classloading of `dynamicThreadPredicate` (since classloading is blocking) + dynamicThreadPredicate.test(Thread.currentThread()); + BlockHoundRuntime.dynamicThreadPredicate = dynamicThreadPredicate; + + // Eagerly trigger the classloading of `threadPredicate` (since classloading is blocking) + threadPredicate.test(Thread.currentThread()); + BlockHoundRuntime.threadPredicate = threadPredicate; + onBlockingMethod = originalOnBlockingMethod; }