8365066: RecordingStream and RemoteRecordingStream do not terminate when the associated Recording is stopped or closed externally #26710

chiroito · 2025-08-09T01:33:29Z

Fix an issue where RecordingStream and RemoteRecordingStream do not stop when their underlying the local/remote recording stop.

Introduce an internal EventSource abstraction to unify control across local (PlatformRecording) and remote (FlightRecorderMXBean) recordings, ensuring consistent propagation of stop/close and stop time to the consumer streams.

Tests updated to validate lifecycle consistency:
Verifies that stopping/closing the underlying recording (local and remote) properly stops/closes the corresponding stream.

Test: jdk/jdk/jfr

Progress

Change must be properly reviewed (1 review required, with at least 1 Reviewer)
Change must not contain extraneous whitespace
Commit message must refer to an issue

Issue

JDK-8365066: RecordingStream and RemoteRecordingStream do not terminate when the associated Recording is stopped or closed externally (Bug - P4)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/26710/head:pull/26710
$ git checkout pull/26710

Update a local copy of the PR:
$ git checkout pull/26710
$ git pull https://git.openjdk.org/jdk.git pull/26710/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 26710

View PR using the GUI difftool:
$ git pr show -t 26710

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/26710.diff

…hen the associated Recording is stopped or closed externally

bridgekeeper · 2025-08-09T01:34:03Z

👋 Welcome back cito! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

openjdk · 2025-08-09T01:35:48Z

❗ This change is not yet ready to be integrated.
See the Progress checklist in the description for automated requirements.

openjdk · 2025-08-09T01:36:40Z

@chiroito The following label will be automatically applied to this pull request:

hotspot-jfr

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

egahlin · 2025-08-09T04:50:26Z

Did you look into adding a FlightRecorderListener and implementing recordingStateChanged to check if the RecordingState is CLOSED and then call close() on the stream? Or perhaps it would be better to set a flag and let the thread running the stream close itself after the next flush is complete.

For a RemoteRecordingStream there is a JMX notification that can be listened on.

turbanoff · 2025-08-12T06:53:55Z

test/jdk/jdk/jfr/api/consumer/recordingstream/TestClosedRecording.java

+    private static class SendEventListener implements FlightRecorderListener {
+        @Override
+        public void recordingStateChanged(Recording recording) {
+            if(recording.getState() == RecordingState.RUNNING){


Suggested change

if(recording.getState() == RecordingState.RUNNING){

if (recording.getState() == RecordingState.RUNNING){

turbanoff · 2025-08-12T06:54:05Z

test/jdk/jdk/jfr/jmx/streaming/TestClosedRecording.java

+    private static class SendEventListener implements FlightRecorderListener {
+        @Override
+        public void recordingStateChanged(Recording recording) {
+            if(recording.getState() == RecordingState.RUNNING){


Suggested change

if(recording.getState() == RecordingState.RUNNING){

if (recording.getState() == RecordingState.RUNNING){

turbanoff · 2025-08-12T06:54:20Z

src/jdk.management.jfr/share/classes/jdk/management/jfr/RemoteRecordingStream.java

@@ -469,7 +467,9 @@ public void close() {
        ManagementSupport.setOnChunkCompleteHandler(stream, null);
        stream.close();
        try {
-            mbean.closeRecording(recordingId);
+            if(eventSource.getState() != RecordingState.CLOSED) {


Suggested change

if(eventSource.getState() != RecordingState.CLOSED) {

if (eventSource.getState() != RecordingState.CLOSED) {

turbanoff · 2025-08-12T06:54:38Z

src/jdk.jfr/share/classes/jdk/jfr/internal/consumer/LocalRecordingEventSource.java

+    }
+
+    @Override
+    public RecordingState getState()  {


Suggested change

public RecordingState getState() {

public RecordingState getState() {

turbanoff · 2025-08-12T06:54:56Z

src/jdk.jfr/share/classes/jdk/jfr/internal/consumer/EventDirectoryStream.java

@@ -169,6 +167,19 @@ protected void processRecursionSafe() throws IOException {
                                     "ns (epoch), parser at " + lastFlush + "ns (epoch).");
                        return;
                    }
+
+                    if(!barrier.used()) {


Suggested change

if(!barrier.used()) {

if (!barrier.used()) {

turbanoff · 2025-08-12T06:55:04Z

src/jdk.jfr/share/classes/jdk/jfr/internal/consumer/EventDirectoryStream.java

-                if (isRecordingStream()) {
-                    if (recording.getState() == RecordingState.STOPPED && !barrier.used()) {
-                        logStreamEnd("recording stopped externally.");
+                if(!barrier.used()) {


Suggested change

if(!barrier.used()) {

if (!barrier.used()) {

turbanoff · 2025-08-12T08:08:30Z

src/jdk.jfr/share/classes/jdk/jfr/internal/consumer/EventDirectoryStream.java

+                    if (state == RecordingState.CLOSED){
+                        logStreamEnd("Event source is closed externally");
+                        return;
+                    }else if(state == RecordingState.STOPPED) {


Suggested change

}else if(state == RecordingState.STOPPED) {

} else if (state == RecordingState.STOPPED) {

turbanoff · 2025-08-12T08:08:43Z

test/jdk/jdk/jfr/api/consumer/recordingstream/TestStoppedRecording.java

+    private static class SendEventListener implements  FlightRecorderListener{
+        @Override
+        public void recordingStateChanged(Recording recording) {
+            if(recording.getState() == RecordingState.RUNNING){


Suggested change

if(recording.getState() == RecordingState.RUNNING){

if (recording.getState() == RecordingState.RUNNING){

turbanoff · 2025-08-12T08:08:56Z

test/jdk/jdk/jfr/jmx/streaming/TestStoppedRecording.java

+    private static class SendEventListener implements FlightRecorderListener {
+        @Override
+        public void recordingStateChanged(Recording recording) {
+            if(recording.getState() == RecordingState.RUNNING){


Suggested change

if(recording.getState() == RecordingState.RUNNING){

if (recording.getState() == RecordingState.RUNNING){

turbanoff · 2025-08-12T08:09:20Z

test/jdk/jdk/jfr/api/consumer/recordingstream/TestStoppedRecording.java

@@ -40,21 +41,44 @@
 */
 public class TestStoppedRecording {

+    private static class SendEventListener implements  FlightRecorderListener{


Suggested change

private static class SendEventListener implements FlightRecorderListener{

private static class SendEventListener implements FlightRecorderListener {

turbanoff · 2025-08-12T08:09:41Z

...jdk.management.jfr/share/classes/jdk/management/jfr/internal/RemoteRecordingEventSource.java

+    @Override
+    public long getStopTime() {
+        Optional<RecordingInfo> recordingInfo = mbean.getRecordings().stream().filter(r -> r.getId() == recordingId).findFirst();
+        if(recordingInfo.isEmpty()){


Suggested change

if(recordingInfo.isEmpty()){

if (recordingInfo.isEmpty()){

turbanoff · 2025-08-12T08:09:56Z

...jdk.management.jfr/share/classes/jdk/management/jfr/internal/RemoteRecordingEventSource.java

+    }
+
+    @Override
+    public RecordingState getState()  {


Suggested change

public RecordingState getState() {

public RecordingState getState() {

turbanoff · 2025-08-12T08:10:03Z

...jdk.management.jfr/share/classes/jdk/management/jfr/internal/RemoteRecordingEventSource.java

+    @Override
+    public RecordingState getState()  {
+        Optional<RecordingInfo> recordingInfo = mbean.getRecordings().stream().filter(r -> r.getId() == recordingId).findFirst();
+        if(recordingInfo.isEmpty()){


Suggested change

if(recordingInfo.isEmpty()){

if (recordingInfo.isEmpty()){

turbanoff · 2025-08-12T08:10:10Z

...jdk.management.jfr/share/classes/jdk/management/jfr/internal/RemoteRecordingEventSource.java

+    public RecordingState getState()  {
+        Optional<RecordingInfo> recordingInfo = mbean.getRecordings().stream().filter(r -> r.getId() == recordingId).findFirst();
+        if(recordingInfo.isEmpty()){
+            return  RecordingState.CLOSED;


Suggested change

return RecordingState.CLOSED;

return RecordingState.CLOSED;

chiroito · 2025-08-12T12:21:39Z

@egahlin That sounds like a good idea. I’ll give it a try.

8365066: RecordingStream and RemoteRecordingStream do not terminate w…

27b52e6

…hen the associated Recording is stopped or closed externally

openjdk bot added the hotspot-jfr [email protected] label Aug 9, 2025

turbanoff reviewed Aug 12, 2025

View reviewed changes

	if(recording.getState() == RecordingState.RUNNING){
	if (recording.getState() == RecordingState.RUNNING){

	if(eventSource.getState() != RecordingState.CLOSED) {
	if (eventSource.getState() != RecordingState.CLOSED) {

	public RecordingState getState() {
	public RecordingState getState() {

	}else if(state == RecordingState.STOPPED) {
	} else if (state == RecordingState.STOPPED) {

	private static class SendEventListener implements FlightRecorderListener{
	private static class SendEventListener implements FlightRecorderListener {

8365066: RecordingStream and RemoteRecordingStream do not terminate when the associated Recording is stopped or closed externally #26710

Are you sure you want to change the base?

8365066: RecordingStream and RemoteRecordingStream do not terminate when the associated Recording is stopped or closed externally #26710

Conversation

chiroito commented Aug 9, 2025 • edited by openjdk bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Progress

Issue

Reviewing

Uh oh!

bridgekeeper bot commented Aug 9, 2025

Uh oh!

openjdk bot commented Aug 9, 2025

Uh oh!

openjdk bot commented Aug 9, 2025

Uh oh!

egahlin commented Aug 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

turbanoff Aug 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chiroito commented Aug 12, 2025

Uh oh!

Uh oh!

chiroito commented Aug 9, 2025 •

edited by openjdk bot

Loading

egahlin commented Aug 9, 2025 •

edited

Loading

turbanoff Aug 12, 2025 •

edited

Loading