From 5cb68eab1e8c114d4a4937e2d43d8516522c4c64 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Fri, 16 Sep 2022 12:22:09 +0100 Subject: [PATCH] Remove HotSpot specifics from HeapDumper strategy interface Closes gh-27533 --- ...HeapDumpWebEndpointDocumentationTests.java | 8 ++- .../management/HeapDumpWebEndpoint.java | 57 ++++++++++--------- .../management/HeapDumpWebEndpointTests.java | 6 +- ...eapDumpWebEndpointWebIntegrationTests.java | 14 ++--- 4 files changed, 47 insertions(+), 38 deletions(-) diff --git a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/endpoint/web/documentation/HeapDumpWebEndpointDocumentationTests.java b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/endpoint/web/documentation/HeapDumpWebEndpointDocumentationTests.java index 6e84c73fe35..cf159838b56 100644 --- a/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/endpoint/web/documentation/HeapDumpWebEndpointDocumentationTests.java +++ b/spring-boot-project/spring-boot-actuator-autoconfigure/src/test/java/org/springframework/boot/actuate/autoconfigure/endpoint/web/documentation/HeapDumpWebEndpointDocumentationTests.java @@ -16,7 +16,9 @@ package org.springframework.boot.actuate.autoconfigure.endpoint.web.documentation; +import java.io.File; import java.io.FileWriter; +import java.nio.file.Files; import java.util.Map; import org.junit.jupiter.api.Test; @@ -66,7 +68,11 @@ class HeapDumpWebEndpointDocumentationTests extends MockMvcEndpointDocumentation @Override protected HeapDumper createHeapDumper() { - return (file, live) -> FileCopyUtils.copy("<>", new FileWriter(file)); + return (live) -> { + File file = Files.createTempFile("heap-", ".hprof").toFile(); + FileCopyUtils.copy("<>", new FileWriter(file)); + return file; + }; } }; diff --git a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/management/HeapDumpWebEndpoint.java b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/management/HeapDumpWebEndpoint.java index ec4cb401b41..ab608ee2244 100644 --- a/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/management/HeapDumpWebEndpoint.java +++ b/spring-boot-project/spring-boot-actuator/src/main/java/org/springframework/boot/actuate/management/HeapDumpWebEndpoint.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2021 the original author or authors. + * Copyright 2012-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -77,7 +77,7 @@ public class HeapDumpWebEndpoint { try { if (this.lock.tryLock(this.timeout, TimeUnit.MILLISECONDS)) { try { - return new WebEndpointResponse<>(dumpHeap((live != null) ? live : true)); + return new WebEndpointResponse<>(dumpHeap(live)); } finally { this.lock.unlock(); @@ -96,29 +96,14 @@ public class HeapDumpWebEndpoint { return new WebEndpointResponse<>(WebEndpointResponse.STATUS_TOO_MANY_REQUESTS); } - private Resource dumpHeap(boolean live) throws IOException, InterruptedException { + private Resource dumpHeap(Boolean live) throws IOException, InterruptedException { if (this.heapDumper == null) { this.heapDumper = createHeapDumper(); } - File file = createTempFile(); - this.heapDumper.dumpHeap(file, live); + File file = this.heapDumper.dumpHeap(live); return new TemporaryFileSystemResource(file); } - private File createTempFile() throws IOException { - String date = DateTimeFormatter.ofPattern("yyyy-MM-dd-HH-mm").format(LocalDateTime.now()); - File file = File.createTempFile("heap-" + date, "." + determineDumpSuffix()); - file.delete(); - return file; - } - - private String determineDumpSuffix() { - if (this.heapDumper instanceof OpenJ9DiagnosticsMXBeanHeapDumper) { - return "phd"; - } - return "hprof"; - } - /** * Factory method used to create the {@link HeapDumper}. * @return the heap dumper to use @@ -140,14 +125,17 @@ public class HeapDumpWebEndpoint { protected interface HeapDumper { /** - * Dump the current heap to the specified file. - * @param file the file to dump the heap to + * Dump the current heap to a file. * @param live if only live objects (i.e. objects that are reachable from - * others) should be dumped + * others) should be dumped. May be {@code null} to use a JVM-specific default. + * @return the file containing the heap dump * @throws IOException on IO error * @throws InterruptedException on thread interruption + * @throws IllegalArgumentException if live is non-null and is not supported by + * the JVM + * @since 3.0.0 */ - void dumpHeap(File file, boolean live) throws IOException, InterruptedException; + File dumpHeap(Boolean live) throws IOException, InterruptedException; } @@ -177,8 +165,18 @@ public class HeapDumpWebEndpoint { } @Override - public void dumpHeap(File file, boolean live) { - ReflectionUtils.invokeMethod(this.dumpHeapMethod, this.diagnosticMXBean, file.getAbsolutePath(), live); + public File dumpHeap(Boolean live) throws IOException { + File file = createTempFile(); + ReflectionUtils.invokeMethod(this.dumpHeapMethod, this.diagnosticMXBean, file.getAbsolutePath(), + (live != null) ? live : true); + return file; + } + + private File createTempFile() throws IOException { + String date = DateTimeFormatter.ofPattern("yyyy-MM-dd-HH-mm").format(LocalDateTime.now()); + File file = File.createTempFile("heap-" + date, ".hprof"); + file.delete(); + return file; } } @@ -209,8 +207,13 @@ public class HeapDumpWebEndpoint { } @Override - public void dumpHeap(File file, boolean live) throws IOException, InterruptedException { - ReflectionUtils.invokeMethod(this.dumpHeapMethod, this.diagnosticMXBean, "heap", file.getAbsolutePath()); + public File dumpHeap(Boolean live) throws IOException, InterruptedException { + if (live != null) { + throw new IllegalArgumentException( + "OpenJ9DiagnosticsMXBean does not support live parameter when dumping the heap"); + } + return new File( + (String) ReflectionUtils.invokeMethod(this.dumpHeapMethod, this.diagnosticMXBean, "heap", null)); } } diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/management/HeapDumpWebEndpointTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/management/HeapDumpWebEndpointTests.java index b626414af18..800b7a87f06 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/management/HeapDumpWebEndpointTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/management/HeapDumpWebEndpointTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2021 the original author or authors. + * Copyright 2012-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,6 +16,7 @@ package org.springframework.boot.actuate.management; +import java.nio.file.Files; import java.util.concurrent.CountDownLatch; import org.junit.jupiter.api.Test; @@ -37,9 +38,10 @@ class HeapDumpWebEndpointTests { @Override protected HeapDumper createHeapDumper() { - return (file, live) -> { + return (live) -> { dumpingLatch.countDown(); blockingLatch.await(); + return Files.createTempFile("heap-", ".dump").toFile(); }; } diff --git a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/management/HeapDumpWebEndpointWebIntegrationTests.java b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/management/HeapDumpWebEndpointWebIntegrationTests.java index c6e58f86506..5c2ef476928 100644 --- a/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/management/HeapDumpWebEndpointWebIntegrationTests.java +++ b/spring-boot-project/spring-boot-actuator/src/test/java/org/springframework/boot/actuate/management/HeapDumpWebEndpointWebIntegrationTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2021 the original author or authors. + * Copyright 2012-2022 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,7 +17,7 @@ package org.springframework.boot.actuate.management; import java.io.File; -import java.io.IOException; +import java.nio.file.Files; import java.time.Duration; import java.util.concurrent.TimeUnit; @@ -98,15 +98,13 @@ class HeapDumpWebEndpointWebIntegrationTests { @Override protected HeapDumper createHeapDumper() { - return (file, live) -> { - this.file = file; + return (live) -> { + this.file = Files.createTempFile("heap-", ".dump").toFile(); if (!TestHeapDumpWebEndpoint.this.available) { throw new HeapDumperUnavailableException("Not available", null); } - if (file.exists()) { - throw new IOException("File exists"); - } - FileCopyUtils.copy(TestHeapDumpWebEndpoint.this.heapDump.getBytes(), file); + FileCopyUtils.copy(TestHeapDumpWebEndpoint.this.heapDump.getBytes(), this.file); + return this.file; }; }