Remove HotSpot specifics from HeapDumper strategy interface

Closes gh-27533
spring-projects · Sep 16, 2022 · 5cb68ea · 5cb68ea
1 parent f731279
commit 5cb68ea
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 38 deletions.
diff --git a/...tuate/autoconfigure/endpoint/web/documentation/HeapDumpWebEndpointDocumentationTests.java b/...tuate/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 @@ HeapDumpWebEndpoint endpoint() {
 
 				@Override
 				protected HeapDumper createHeapDumper() {
-					return (file, live) -> FileCopyUtils.copy("<<binary content>>", new FileWriter(file));
+					return (live) -> {
+						File file = Files.createTempFile("heap-", ".hprof").toFile();
+						FileCopyUtils.copy("<<binary content>>", new FileWriter(file));
+						return file;
+					};
 				}
 
 			};

diff --git a/...tuator/src/main/java/org/springframework/boot/actuate/management/HeapDumpWebEndpoint.java b/...tuator/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 WebEndpointResponse<Resource> heapDump(@Nullable Boolean live) {
 		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 WebEndpointResponse<Resource> heapDump(@Nullable Boolean live) {
 		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 @@ protected HeapDumper createHeapDumper() throws HeapDumperUnavailableException {
 	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 <em>live</em> 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 @@ protected HotSpotDiagnosticMXBeanHeapDumper() {
 		}
 
 		@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 @@ private OpenJ9DiagnosticsMXBeanHeapDumper() {
 		}
 
 		@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/...r/src/test/java/org/springframework/boot/actuate/management/HeapDumpWebEndpointTests.java b/...r/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 @@ void parallelRequestProducesTooManyRequestsResponse() throws InterruptedExceptio
 
 			@Override
 			protected HeapDumper createHeapDumper() {
-				return (file, live) -> {
+				return (live) -> {
 					dumpingLatch.countDown();
 					blockingLatch.await();
+					return Files.createTempFile("heap-", ".dump").toFile();
 				};
 			}
 

diff --git a/...a/org/springframework/boot/actuate/management/HeapDumpWebEndpointWebIntegrationTests.java b/...a/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 @@ void reset() {
 
 		@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;
 			};
 		}