8286740: JFR: Active Setting event emitted incorrectly

Reviewed-by: jbachorik
Backport-of: 24cab0af32a1eaa4c594fb2a144386a6b7062981

src/jdk.jfr/share/classes/jdk/jfr/events/ActiveSettingEvent.java

@@ -37,13 +37,6 @@
 @StackTrace(false)
 public final class ActiveSettingEvent extends AbstractJDKEvent {
 
-    public static final ThreadLocal<ActiveSettingEvent> EVENT = new ThreadLocal<ActiveSettingEvent>() {
-        @Override
-        protected ActiveSettingEvent initialValue() {
-            return new ActiveSettingEvent();
-        }
-    };
-
     @Label("Event Id")
     public long id;
 

src/jdk.jfr/share/classes/jdk/jfr/internal/EventControl.java

@@ -290,13 +290,13 @@ void writeActiveSettingEvent() {
         if (!type.isRegistered()) {
             return;
         }
-        ActiveSettingEvent event = ActiveSettingEvent.EVENT.get();
         for (NamedControl nc : namedControls) {
             if (Utils.isSettingVisible(nc.control, type.hasEventHook())) {
                 String value = nc.control.getLastValue();
                 if (value == null) {
                     value = nc.control.getDefaultValue();
                 }
+                ActiveSettingEvent event = new ActiveSettingEvent();
                 event.id = type.getId();
                 event.name = nc.name;
                 event.value = value;

src/jdk.jfr/share/classes/jdk/jfr/internal/MetadataRepository.java

@@ -144,7 +144,7 @@ public synchronized EventType register(Class<? extends jdk.internal.event.Event>
         handler.setRegistered(true);
         typeLibrary.addType(handler.getPlatformEventType());
         if (jvm.isRecording()) {
-            settingsManager.setEventControl(handler.getEventControl());
+            settingsManager.setEventControl(handler.getEventControl(), true);
             settingsManager.updateRetransform(Collections.singletonList((eventClass)));
        }
        setStaleMetadata();
@@ -199,8 +199,8 @@ private EventHandler makeHandler(Class<? extends jdk.internal.event.Event> event
     }
 
 
-    public synchronized void setSettings(List<Map<String, String>> list) {
-        settingsManager.setSettings(list);
+    public synchronized void setSettings(List<Map<String, String>> list, boolean writeSettingEvents) {
+        settingsManager.setSettings(list, writeSettingEvents);
     }
 
     synchronized void disableEvents() {

src/jdk.jfr/share/classes/jdk/jfr/internal/PlatformRecorder.java

@@ -256,7 +256,7 @@ synchronized long start(PlatformRecording recording) {
                 currentChunk.setStartTime(startTime);
             }
             recording.setState(RecordingState.RUNNING);
-            updateSettings();
+            updateSettings(false);
             recording.setStartTime(startTime);
             writeMetaEvents();
         } else {
@@ -275,7 +275,7 @@ synchronized long start(PlatformRecording recording) {
             startTime = Utils.epochNanosToInstant(startNanos);
             recording.setStartTime(startTime);
             recording.setState(RecordingState.RUNNING);
-            updateSettings();
+            updateSettings(false);
             writeMetaEvents();
             if (currentChunk != null) {
                 finishChunk(currentChunk, startTime, recording);
@@ -339,7 +339,7 @@ synchronized void stop(PlatformRecording recording) {
         } else {
             RepositoryChunk newChunk = null;
             RequestEngine.doChunkEnd();
-            updateSettingsButIgnoreRecording(recording);
+            updateSettingsButIgnoreRecording(recording, false);
 
             String path = null;
             if (toDisk) {
@@ -383,19 +383,19 @@ private void disableEvents() {
         MetadataRepository.getInstance().disableEvents();
     }
 
-    void updateSettings() {
-        updateSettingsButIgnoreRecording(null);
+    void updateSettings(boolean writeSettingEvents) {
+        updateSettingsButIgnoreRecording(null, writeSettingEvents);
     }
 
-    void updateSettingsButIgnoreRecording(PlatformRecording ignoreMe) {
+    void updateSettingsButIgnoreRecording(PlatformRecording ignoreMe, boolean writeSettingEvents) {
         List<PlatformRecording> recordings = getRunningRecordings();
         List<Map<String, String>> list = new ArrayList<>(recordings.size());
         for (PlatformRecording r : recordings) {
             if (r != ignoreMe) {
                 list.add(r.getSettings());
             }
         }
-        MetadataRepository.getInstance().setSettings(list);
+        MetadataRepository.getInstance().setSettings(list, writeSettingEvents);
     }
 
 

src/jdk.jfr/share/classes/jdk/jfr/internal/PlatformRecording.java

@@ -468,7 +468,7 @@ public void setSetting(String id, String value) {
         synchronized (recorder) {
             this.settings.put(id, value);
             if (getState() == RecordingState.RUNNING) {
-                recorder.updateSettings();
+                recorder.updateSettings(true);
             }
         }
     }
@@ -489,7 +489,7 @@ private void setSettings(Map<String, String> settings, boolean update) {
         synchronized (recorder) {
             this.settings = new LinkedHashMap<>(settings);
             if (getState() == RecordingState.RUNNING && update) {
-                recorder.updateSettings();
+                recorder.updateSettings(true);
             }
         }
     }

src/jdk.jfr/share/classes/jdk/jfr/internal/SettingsManager.java

@@ -130,7 +130,7 @@ public void finish() {
 
    private Map<String, InternalSetting> availableSettings = new LinkedHashMap<>();
 
-    void setSettings(List<Map<String, String>> activeSettings) {
+    void setSettings(List<Map<String, String>> activeSettings, boolean writeSettingEvents) {
         // store settings so they are available if a new event class is loaded
         availableSettings = createSettingsMap(activeSettings);
         List<EventControl> eventControls = MetadataRepository.getInstance().getEventControls();
@@ -143,7 +143,7 @@ void setSettings(List<Map<String, String>> activeSettings) {
                 Collections.sort(eventControls, (x,y) -> x.getEventType().getName().compareTo(y.getEventType().getName()));
             }
             for (EventControl ec : eventControls) {
-                setEventControl(ec);
+                setEventControl(ec, writeSettingEvents);
             }
         }
         if (JVM.getJVM().getAllowedToDoEventRetransforms()) {
@@ -211,7 +211,7 @@ private Collection<InternalSetting> makeInternalSettings(Map<String, String> rec
       return internals.values();
     }
 
-    void setEventControl(EventControl ec) {
+    void setEventControl(EventControl ec, boolean writeSettingEvents) {
         InternalSetting is = getInternalSetting(ec);
         boolean shouldLog = Logger.shouldLog(LogTag.JFR_SETTING, LogLevel.INFO);
         if (shouldLog) {
@@ -250,7 +250,9 @@ void setEventControl(EventControl ec) {
                 }
             }
         }
-        ec.writeActiveSettingEvent();
+        if (writeSettingEvents) {
+            ec.writeActiveSettingEvent();
+        }
         if (shouldLog) {
             Logger.log(LogTag.JFR_SETTING, LogLevel.INFO, "}");
         }

test/jdk/jdk/jfr/event/runtime/TestActiveSettingEvent.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, 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
@@ -23,8 +23,10 @@
 package jdk.jfr.event.runtime;
 
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
 import jdk.jfr.Configuration;
 import jdk.jfr.Event;
@@ -60,6 +62,7 @@ private static class MyRegistrationEvent extends Event {
     public static void main(String[] args) throws Throwable {
         testDefaultSettings();;
         testProfileSettings();;
+        testOnlyOnce();
         testNewSettings();
         testChangedSetting();
         testUnregistered();
@@ -74,6 +77,39 @@ private static void testDefaultSettings() throws Exception {
         testSettingConfiguration("default");
     }
 
+    private static void testOnlyOnce() throws Exception {
+        Configuration c = Configuration.getConfiguration("default");
+        try (Recording r = new Recording(c)) {
+            r.enable(ACTIVE_SETTING_EVENT_NAME).withStackTrace();
+            r.start();
+            r.stop();
+            Map<String, RecordedEvent> settings = new HashMap<>();
+            List<RecordedEvent> events = Events.fromRecording(r);
+            for (RecordedEvent e : events) {
+                if (e.getEventType().getName().equals(ACTIVE_SETTING_EVENT_NAME)) {
+                    long id = e.getLong("id");
+                    String name = e.getString("name");
+                    String value = e.getString("value");
+                    String s = id + "#" + name + "=" + value;
+                    if (settings.containsKey(s)) {
+                        System.out.println("Event:");
+                        System.out.println(settings.get(s));
+                        System.out.println("Duplicated by:");
+                        System.out.println(e);
+                        String message = "Found duplicated setting '" + s + "'";
+                        for (EventType type : FlightRecorder.getFlightRecorder().getEventTypes()) {
+                            if (type.getId() == id) {
+                                throw new Exception(message+  " for " + type.getName());
+                            }
+                        }
+                        throw new Exception(message);
+                    }
+                    settings.put(s, e);
+                }
+            }
+        }
+    }
+
     private static void testRegistration() throws Exception {
         // Register new
         try (Recording recording = new Recording()) {