Skip to content

Commit

Permalink
8294151: JFR: Unclear exception message when dumping stopped in memor…
Browse files Browse the repository at this point in the history
…y recording

Reviewed-by: mgronlun
  • Loading branch information
egahlin committed Oct 5, 2022
1 parent 8ebebbc commit 13a5000
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 10 deletions.
17 changes: 11 additions & 6 deletions src/jdk.jfr/share/classes/jdk/jfr/Recording.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2016, 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2016, 2022, 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
Expand Down Expand Up @@ -348,7 +348,7 @@ public void close() {

/**
* Returns a clone of this recording, with a new recording ID and name.
*
* <p>
* Clones are useful for dumping data without stopping the recording. After
* a clone is created, the amount of data to copy is constrained
* with the {@link #setMaxAge(Duration)} method and the {@link #setMaxSize(long)}method.
Expand All @@ -364,21 +364,26 @@ public Recording copy(boolean stop) {
/**
* Writes recording data to a file.
* <p>
* Recording must be started, but not necessarily stopped.
* For a dump to succeed, the recording must either be 1) running, or 2) stopped
* and to disk. If the recording is in any other state, an
* {@link IOException} is thrown.
*
* @param destination the location where recording data is written, not
* {@code null}
*
* @throws IOException if the recording can't be copied to the specified
* location
* @throws IOException if recording data can't be copied to the specified
* location, for example, if the recording is closed or the
* destination path is not writable
*
* @throws SecurityException if a security manager exists and the caller doesn't
* have {@code FilePermission} to write to the destination path
*
* @see #getState()
* @see #isToDisk()
*/
public void dump(Path destination) throws IOException {
Objects.requireNonNull(destination, "destination");
internal.dump(new WriteableUserPath(destination));

}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2016, 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2016, 2022, 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
Expand Down Expand Up @@ -321,7 +321,12 @@ public PlatformRecording newSnapshotClone(String reason, Boolean pathToGcRoots)
if (state == RecordingState.DELAYED || state == RecordingState.NEW) {
throw new IOException("Recording \"" + name + "\" (id=" + id + ") has not started, no content to write");
}

if (state == RecordingState.STOPPED) {
if (!isToDisk()) {
throw new IOException("Recording \"" + name + "\" (id=" + id + ")"
+ " is an in memory recording. No data to copy after it has been stopped.");
}
PlatformRecording clone = recorder.newTemporaryRecording();
for (RepositoryChunk r : chunks) {
clone.add(r);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2016, 2021, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2016, 2022, 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
Expand Down Expand Up @@ -136,7 +136,11 @@ public Void run() throws Exception {
} catch (Throwable t) {
// prevent malicious user to propagate exception callback
// in the wrong context
throw new IOException("Unexpected error during I/O operation");
Throwable cause = null;
if (System.getSecurityManager() == null) {
cause = t;
}
throw new IOException("Unexpected error during I/O operation", cause);
} finally {
inPrivileged = false;
}
Expand Down
25 changes: 24 additions & 1 deletion test/jdk/jdk/jfr/api/recording/misc/TestRecordingCopy.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2016, 2018, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2016, 2022, 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
Expand Down Expand Up @@ -29,6 +29,8 @@
import jdk.test.lib.jfr.Events;
import jdk.test.lib.jfr.SimpleEvent;

import java.io.IOException;
import java.nio.file.Paths;
import java.util.List;

/**
Expand Down Expand Up @@ -82,6 +84,27 @@ public static void main(String[] args) throws Exception {
runningCopy.stop();
runningCopy.close();
stoppedCopy.close();

testMemoryCopy();
}

private static void testMemoryCopy() throws Exception {
try (Recording memory = new Recording()) {
memory.setToDisk(false);
memory.enable(SimpleEvent.class);
memory.start();

Recording unstopped = memory.copy(false);
unstopped.dump(Paths.get("unstopped-memory.jfr"));

Recording stopped = memory.copy(true);
try {
stopped.dump(Paths.get("stopped-memory.jfr"));
throw new Exception("Should not be able to dump stopped in memory recording");
} catch (IOException ioe) {
// As expected
}
}
}

/**
Expand Down

1 comment on commit 13a5000

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.