From cf5a25538e09e449ff621562df6529abaa9b3685 Mon Sep 17 00:00:00 2001 From: Erik Gahlin Date: Mon, 4 Aug 2025 09:12:12 +0000 Subject: [PATCH] 8364427: JFR: Possible resource leak in Recording::getStream Reviewed-by: mgronlun --- .../jdk/jfr/internal/ChunkInputStream.java | 40 +++-- .../misc/TestGetStreamWithFailure.java | 146 ++++++++++++++++++ 2 files changed, 170 insertions(+), 16 deletions(-) create mode 100644 test/jdk/jdk/jfr/api/recording/misc/TestGetStreamWithFailure.java diff --git a/src/jdk.jfr/share/classes/jdk/jfr/internal/ChunkInputStream.java b/src/jdk.jfr/share/classes/jdk/jfr/internal/ChunkInputStream.java index 4c8913e96a0..3e95932b197 100644 --- a/src/jdk.jfr/share/classes/jdk/jfr/internal/ChunkInputStream.java +++ b/src/jdk.jfr/share/classes/jdk/jfr/internal/ChunkInputStream.java @@ -49,7 +49,9 @@ final class ChunkInputStream extends InputStream { } this.chunks = l.iterator(); - nextStream(); + if (!nextStream()) { + throw new IOException("Recording data missing on disk."); + } } @Override @@ -62,13 +64,18 @@ final class ChunkInputStream extends InputStream { } private boolean nextStream() throws IOException { - if (!nextChunk()) { - return false; + while (nextChunk()) { + try { + stream = new BufferedInputStream(Files.newInputStream(currentChunk.getFile())); + unstreamedSize -= currentChunk.getSize(); + return true; + } catch (IOException e) { + Logger.log(LogTag.JFR, LogLevel.INFO, "Could not open chunk file for stream: " + e.getMessage() + ". Skipping."); + // Release chunk if it can't be found/accessed. + closeChunk(); + } } - - stream = new BufferedInputStream(Files.newInputStream(currentChunk.getFile())); - unstreamedSize -= currentChunk.getSize(); - return true; + return false; } private boolean nextChunk() { @@ -126,11 +133,14 @@ final class ChunkInputStream extends InputStream { } private void closeStream() throws IOException { - if (stream != null) { - stream.close(); - stream = null; + try { + if (stream != null) { + stream.close(); + stream = null; + } + } finally { + closeChunk(); } - closeChunk(); } private void closeChunk() { @@ -143,11 +153,9 @@ final class ChunkInputStream extends InputStream { @Override public void close() throws IOException { closeStream(); - while (currentChunk != null) { - closeChunk(); - if (!nextChunk()) { - return; - } + while (chunks.hasNext()) { + RepositoryChunk c = chunks.next(); + c.release(); } } } diff --git a/test/jdk/jdk/jfr/api/recording/misc/TestGetStreamWithFailure.java b/test/jdk/jdk/jfr/api/recording/misc/TestGetStreamWithFailure.java new file mode 100644 index 00000000000..d294b06270e --- /dev/null +++ b/test/jdk/jdk/jfr/api/recording/misc/TestGetStreamWithFailure.java @@ -0,0 +1,146 @@ +/* + * Copyright (c) 2025, Oracle and/or its affiliates. All rights reserved. + * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. + * + * This code is free software; you can redistribute it and/or modify it + * under the terms of the GNU General Public License version 2 only, as + * published by the Free Software Foundation. + * + * This code is distributed in the hope that it will be useful, but WITHOUT + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License + * version 2 for more details (a copy is included in the LICENSE file that + * accompanied this code). + * + * You should have received a copy of the GNU General Public License version + * 2 along with this work; if not, write to the Free Software Foundation, + * Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA. + * + * Please contact Oracle, 500 Oracle Parkway, Redwood Shores, CA 94065 USA + * or visit www.oracle.com if you need additional information or have any + * questions. + */ + +package jdk.jfr.api.recording.misc; + +import java.io.IOException; +import java.io.InputStream; +import java.nio.file.Files; +import java.nio.file.Path; +import java.text.ParseException; +import java.util.List; +import java.util.function.BiConsumer; + +import jdk.jfr.Configuration; +import jdk.jfr.Event; +import jdk.jfr.Recording; + +/** + * @test + * @summary Verify that resources are not leaked in case of failure + * @requires vm.flagless + * @requires vm.hasJFR + * @library /test/lib + * @run main/othervm jdk.jfr.api.recording.misc.TestGetStreamWithFailure + */ +public class TestGetStreamWithFailure { + private interface TestCase { + void execute(Recording recording, List chunks) throws Exception; + } + + private static class FillerEvent extends Event { + } + + public static void main(String[] args) throws Exception { + testMissingChunk(); + testClosedStream(); + testNoChunks(); + } + + // Simulates a user deleting all chunks for a recording + private static void testNoChunks() throws Exception { + testStream((r, chunks) -> { + deleteAllChunks(); + try { + InputStream is = r.getStream(null, null); + throw new Exception("Expected exception when all chunks are missing"); + } catch (IOException ioe) { + if (!ioe.getMessage().equals("Recording data missing on disk.")) { + throw new Exception("Unexpected exception: " + ioe.getMessage()); + } + } + }); + } + + private static void deleteAllChunks() throws Exception { + for (Path chunk : findChunks()) { + Files.delete(chunk); + } + } + + // Simulates a user deleting a single chunk file on disk. + private static void testMissingChunk() throws Exception { + testStream((r, chunks) -> { + Files.delete(chunks.get(1)); + try (InputStream is = r.getStream(null, null)) { + is.readAllBytes(); + } + }); + } + + // Simulates a user closing a stream before all the data has been read, for + // example, if InputStream::read throws an IOException. + private static void testClosedStream() throws Exception { + testStream((r, chunks) -> { + int size = (int) (r.getSize()); + try (InputStream is = r.getStream(null, null)) { + is.readNBytes(size / 2); + } + }); + } + + private static void testStream(TestCase testCase) throws Exception { + try (Recording r = createRecordingData()) { + List chunks = findChunks(); + if (chunks.size() < 3) { + throw new Exception("Expected recording to have at least three chunks"); + } + testCase.execute(r, chunks); + r.close(); + if (!findChunks().isEmpty()) { + throw new Exception("Chunks left behind."); + } + deleteAllChunks(); + } + } + + private static List findChunks() throws Exception { + String repository = System.getProperty("jdk.jfr.repository"); + if (repository == null) { + throw new Exception("No system property for JFR repository"); + } + Path dir = Path.of(repository); + return Files.walk(dir).filter(p -> p.toString().endsWith(".jfr")).toList(); + } + + private static Recording createRecordingData() throws IOException, ParseException, InterruptedException { + Configuration c = Configuration.getConfiguration("default"); + Recording r = new Recording(); + r.start(); + emitEvents(); // Chunk 1 + try (Recording s = new Recording()) { + s.start(); + emitEvents(); // Chunk 2 + } + emitEvents(); // Chunk 3 + r.stop(); + return r; + } + + private static void emitEvents() throws InterruptedException { + for (int i = 0; i < 100_000; i++) { + FillerEvent e = new FillerEvent(); + e.commit(); + } + } +}