8373439: Deadlock between flight recorder & VM shutdown

Reviewed-by: mgronlun
This commit is contained in:
Erik Gahlin 2026-04-29 16:30:22 +00:00
parent 290cdb7258
commit 004d0ecf86
9 changed files with 310 additions and 31 deletions

View File

@ -151,8 +151,8 @@ public abstract class EventSettings {
}
@Override
public Recording newRecording(RecordingState state) {
return new Recording(state, Map.of());
public Recording newRecording(Boolean register) {
return new Recording(register, Map.of());
}
@Override

View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2016, 2025, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2016, 2026, 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
@ -99,8 +99,12 @@ public final class FlightRecorder {
*/
public Recording takeSnapshot() {
Recording snapshot = new Recording();
snapshot.setName("Snapshot");
internal.fillWithRecordedData(snapshot.getInternal(), null);
snapshot.getInternal().setName("Snapshot", false);
synchronized (internal) {
if (!internal.isDestroyed()) {
internal.fillWithRecordedData(snapshot.getInternal(), null);
}
}
return snapshot;
}

View File

@ -35,7 +35,6 @@ import java.util.HashMap;
import java.util.Map;
import java.util.Objects;
import jdk.jfr.RecordingState;
import jdk.jfr.internal.PlatformRecorder;
import jdk.jfr.internal.PlatformRecording;
import jdk.jfr.internal.Type;
@ -101,16 +100,16 @@ public final class Recording implements Closeable {
* @since 11
*/
public Recording(Map<String, String> settings) {
this(RecordingState.NEW, settings);
this(null, settings);
}
// package private
Recording(RecordingState state, Map<String, String> settings) {
Recording(Boolean register, Map<String, String> settings) {
Objects.requireNonNull(settings, "settings");
Map<String, String> sanitized = Utils.sanitizeNullFreeStringMap(settings);
PlatformRecorder r = FlightRecorder.getFlightRecorder().getInternal();
synchronized (r) {
this.internal = r.newRecording(state, sanitized);
this.internal = r.newRecording(register, sanitized);
this.internal.setRecording(this);
if (internal.getRecording() != this) {
throw new InternalError("Internal recording not properly setup");
@ -503,7 +502,7 @@ public final class Recording implements Closeable {
*/
public void setName(String name) {
Objects.requireNonNull(name, "name");
internal.setName(name);
internal.setName(name, true);
}
/**

View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2019, 2025, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2019, 2026, 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
@ -41,6 +41,7 @@ import jdk.jfr.EventSettings;
import jdk.jfr.EventType;
import jdk.jfr.Recording;
import jdk.jfr.RecordingState;
import jdk.jfr.internal.PlatformRecorder;
import jdk.jfr.internal.PlatformRecording;
import jdk.jfr.internal.PrivateAccess;
import jdk.jfr.internal.util.Utils;
@ -97,9 +98,9 @@ public final class RecordingStream implements AutoCloseable, EventStream {
private RecordingStream(Map<String, String> settings) {
this.recording = new Recording();
this.creationTime = Instant.now();
this.recording.setName("Recording Stream: " + creationTime);
try {
PlatformRecording pr = PrivateAccess.getInstance().getPlatformRecording(recording);
pr.setName("Recording Stream: " + creationTime, false);
this.directoryStream = new EventDirectoryStream(
null,
pr,
@ -113,6 +114,12 @@ public final class RecordingStream implements AutoCloseable, EventStream {
if (!settings.isEmpty()) {
recording.setSettings(settings);
}
PlatformRecorder recorder = PrivateAccess.getInstance().getPlatformRecorder();
synchronized (recorder) {
if (recorder.isDestroyed()) {
close();
}
}
}
private List<Configuration> configurations() {

View File

@ -67,6 +67,7 @@ public final class PlatformRecorder {
private long recordingCounter = 0;
private RepositoryChunk currentChunk;
private boolean runPeriodicTask;
private boolean destroyed;
public PlatformRecorder() throws Exception {
repository = Repository.getRepository();
@ -82,8 +83,18 @@ public final class PlatformRecorder {
Runtime.getRuntime().addShutdownHook(shutdownHook);
}
public synchronized PlatformRecording newRecording(RecordingState state, Map<String, String> settings) {
return newRecording(state, settings, ++recordingCounter);
public synchronized PlatformRecording newRecording(Boolean register, Map<String, String> settings) {
long id = ++recordingCounter;
if (register == null) {
if (isDestroyed()) {
PlatformRecording r = newRecording(false, settings, id);
r.setState(RecordingState.CLOSED);
return r;
} else {
return newRecording(true, settings, id);
}
}
return newRecording(register, settings, id);
}
// To be used internally when doing dumps.
@ -92,15 +103,15 @@ public final class PlatformRecorder {
if(!Thread.holdsLock(this)) {
throw new InternalError("Caller must have recorder lock");
}
return newRecording(RecordingState.NEW, new HashMap<>(), 0);
return newRecording(true, new HashMap<>(), 0);
}
private synchronized PlatformRecording newRecording(RecordingState state, Map<String, String> settings, long id) {
private synchronized PlatformRecording newRecording(boolean register, Map<String, String> settings, long id) {
PlatformRecording recording = new PlatformRecording(this, id);
if (!settings.isEmpty()) {
recording.setSettings(settings);
}
if (state != RecordingState.CLOSED) {
if (register) {
recordings.add(recording);
}
return recording;
@ -177,8 +188,13 @@ public final class PlatformRecorder {
}
}
}
writeReports();
destroyed = true;
// This will prevent further interaction with recordings after JFR has been destroyed.
for (PlatformRecording p : getRecordings()) {
p.setState(RecordingState.CLOSED);
}
recordings.clear();
JDKEvents.remove();
if (JVMSupport.hasJFR()) {
@ -190,11 +206,29 @@ public final class PlatformRecorder {
repository.clear();
}
// Not synchronized. Caller must hold recorder lock to avoid races.
public boolean isDestroyed() {
if (!Thread.holdsLock(this)) {
throw new InternalError("Caller must have recorder lock");
}
return destroyed;
}
private void writeReports() {
for (PlatformRecording recording : getRecordings()) {
if (recording.isToDisk() && recording.getState() == RecordingState.STOPPED) {
for (Report report : recording.getReports()) {
report.print(recording.getStartTime(), recording.getStopTime());
try {
report.print(recording.getStartTime(), recording.getStopTime());
} catch (Exception e) {
StringBuilder message = new StringBuilder();
message.append("Could not generate report-on-exit for view ");
message.append(report.name());
message.append(" (recording ");
message.append(recording.getName()).append(":").append(recording.getId());
message.append("). Unexpected error: ").append(e.toString());
Logger.log(JFR, WARN, message.toString());
}
}
}
}
@ -547,15 +581,15 @@ public final class PlatformRecorder {
}
synchronized Recording newCopy(PlatformRecording r, boolean stop) {
PrivateAccess pr = PrivateAccess.getInstance();
boolean closed = r.getState() == RecordingState.CLOSED;
Recording newRec = closed ? pr.newRecording(RecordingState.CLOSED) : new Recording();
PlatformRecording copy = pr.getPlatformRecording(newRec);
PrivateAccess access = PrivateAccess.getInstance();
boolean register = !isDestroyed() && r.getState() != RecordingState.CLOSED;
Recording newRec = access.newRecording(register);
PlatformRecording copy = access.getPlatformRecording(newRec);
copy.setSettings(r.getSettings());
copy.setMaxAge(r.getMaxAge());
copy.setMaxSize(r.getMaxSize());
copy.setDumpOnExit(r.getDumpOnExit());
copy.setName("Clone of " + r.getName());
copy.setName("Clone of " + r.getName(), false);
copy.setToDisk(r.isToDisk());
copy.setInternalDuration(r.getDuration());
copy.setStartTime(r.getStartTime());

View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2016, 2025, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2016, 2026, 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
@ -353,7 +353,7 @@ public final class PlatformRecording implements AutoCloseable {
// Recording is RUNNING, create a clone
PlatformRecording clone = recorder.newTemporaryRecording();
clone.setShouldWriteActiveRecordingEvent(false);
clone.setName(getName());
clone.setName(getName(), false);
clone.setToDisk(true);
clone.setMaxAge(getMaxAge());
clone.setMaxSize(getMaxSize());
@ -425,7 +425,7 @@ public final class PlatformRecording implements AutoCloseable {
}
}
void setState(RecordingState state) {
public void setState(RecordingState state) {
synchronized (recorder) {
this.state = state;
}
@ -449,9 +449,11 @@ public final class PlatformRecording implements AutoCloseable {
}
}
public void setName(String name) {
public void setName(String name, boolean checkClosed) {
synchronized (recorder) {
ensureNotClosed();
if (checkClosed) {
ensureNotClosed();
}
this.name = name;
}
}

View File

@ -33,7 +33,6 @@ import jdk.jfr.Configuration;
import jdk.jfr.EventSettings;
import jdk.jfr.EventType;
import jdk.jfr.Recording;
import jdk.jfr.RecordingState;
import jdk.jfr.SettingDescriptor;
import jdk.jfr.ValueDescriptor;
import jdk.jfr.internal.management.EventSettingsModifier;
@ -97,7 +96,7 @@ public abstract class PrivateAccess {
public abstract PlatformRecorder getPlatformRecorder();
public abstract Recording newRecording(RecordingState state);
public abstract Recording newRecording(Boolean register);
public abstract EventSettings newEventSettings(EventSettingsModifier esm);

View File

@ -0,0 +1,190 @@
/*
* Copyright (c) 2026, 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.jvm;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import jdk.jfr.Recording;
import jdk.jfr.FlightRecorder;
import jdk.jfr.RecordingState;
import jdk.jfr.consumer.RecordingStream;
/**
* Test application that tries to interact with JFR after shutdown.
*/
@SuppressWarnings("resource")
public class AfterShutdown {
private static boolean fail;
public static void main(String... args) throws Exception {
Recording fresh = new Recording();
Recording started = new Recording();
started.start();
Recording stopped = new Recording();
stopped.start();
stopped.stop();
Recording closed = new Recording();
closed.close();
Recording fullCycle = new Recording();
fullCycle.start();
fullCycle.stop();
fullCycle.close();
Recording copyable = new Recording();
copyable.start();
Runtime.getRuntime().addShutdownHook(new Thread() {
public void run() {
try {
awaitRepositoryRemoved();
assertClosedBehavior(fresh, "new recording");
assertClosedBehavior(started, "started recording");
assertClosedBehavior(stopped, "stopped recording");
assertClosedBehavior(closed, "closed recording");
assertClosedBehavior(fullCycle, "full-cycle recording");
testCopy(copyable);
testSnapshot();
testLateRecording();
testLateRecordingStream();
} catch (Throwable t) {
fail = true;
t.printStackTrace();
}
if (!fail) {
System.out.println("PASS");
}
}
});
}
private static void testLateRecording() {
Recording r = new Recording();
assertClosedBehavior(r, "late recording");
}
private static void testCopy(Recording r) {
try (Recording copy = r.copy(false)) {
assertClosedBehavior(copy, "copied recording with stopped=true");
}
try (Recording copy = r.copy(true)) {
assertClosedBehavior(copy, "copied recording with stopped=false");
}
}
private static void testSnapshot() {
try (Recording snapshot = FlightRecorder.getFlightRecorder().takeSnapshot()) {
assertClosedBehavior(snapshot, "snapshot");
}
}
public static void assertClosedBehavior(Recording recording, String kind) {
assertClosed(recording);
try {
recording.start();
fail("Expected IllegalStateException if " + kind + " is started after shutdown.");
} catch (IllegalStateException ise) {
// As expected
}
try {
recording.dump(Path.of("file.jfr"));
fail("Expected IOException if " + kind + " is dumped after shutdown");
} catch (IOException ioe) {
// As expected
}
try {
recording.stop();
fail("Expected IllegalStateException if " + kind + " is stopped after shutdown");
} catch (IllegalStateException ioe) {
// As expected
}
try {
recording.close();
} catch (Exception e) {
e.printStackTrace();
fail("Should be able to close " + kind + " after shutdown.");
}
}
private static void testLateRecordingStream() {
RecordingStream rs = new RecordingStream();
assertClosedBehavior(rs, "late recording stream");
}
public static void assertClosedBehavior(RecordingStream rs, String kind) {
try {
rs.start();
fail("Expected IllegalStateException if " + kind + " is started after shutdown.");
} catch (IllegalStateException ise) {
// As expected
}
try {
rs.dump(Path.of("file.jfr"));
fail("Expected IOException if " + kind + " is dumped after shutdown");
} catch (IOException ioe) {
// As expected
}
try {
rs.stop();
fail("Expected IllegalStateException if " + kind + " is stopped after shutdown");
} catch (IllegalStateException ioe) {
// As expected
}
try {
rs.close();
} catch (Exception e) {
e.printStackTrace();
fail("Should be able to close " + kind + " after shutdown.");
}
}
private static void awaitRepositoryRemoved() {
String path = System.getProperty("jdk.jfr.repository");
Path repository = Path.of(path);
while (Files.exists(repository)) {
System.out.println("Repository still exist. Waiting 100 ms ...");
try {
Thread.sleep(100);
} catch (InterruptedException ie) {
Thread.currentThread().interrupt();
}
}
System.out.println("Repository removed.");
}
private static void assertClosed(Recording r) {
if (r.getState() != RecordingState.CLOSED) {
fail("Recording was in state " + r.getState() + " but expected it to be CLOSED");
}
}
private static void fail(String message) {
System.out.println("FAIL: " + message);
fail = true;
}
}

View File

@ -0,0 +1,44 @@
/*
* Copyright (c) 2026, 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.jvm;
import jdk.test.lib.process.ProcessTools;
/**
* @test TestAfterShutdown
* @requires vm.flagless
* @summary Checks that API interactions with JFR after shutdown works as expected
* @requires vm.hasJFR
* @library /test/lib
* @build jdk.jfr.jvm.AfterShutdown
* @run main/othervm jdk.jfr.jvm.TestAfterShutdown
*/
public class TestAfterShutdown {
public static void main(String... args) throws Exception {
var pb = ProcessTools.createTestJavaProcessBuilder(AfterShutdown.class.getName());
var result = ProcessTools.executeProcess(pb);
result.shouldHaveExitValue(0);
result.shouldNotContain("FAIL");
result.shouldContain("PASS");
}
}