8286740: JFR: Active Setting event emitted incorrectly

Reviewed-by: mgronlun
This commit is contained in:
Erik Gahlin 2022-05-16 19:09:02 +00:00
parent a31130fd40
commit 24cab0af32
7 changed files with 55 additions and 24 deletions

View File

@ -37,13 +37,6 @@ import jdk.jfr.internal.Type;
@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;

View File

@ -283,13 +283,13 @@ public final class EventControl {
if (!type.isRegistered()) {
return;
}
ActiveSettingEvent event = ActiveSettingEvent.EVENT.get();
for (NamedControl nc : namedControls) {
if (Utils.isSettingVisible(nc.control, type.hasEventHook()) && type.isVisible()) {
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;

View File

@ -158,7 +158,7 @@ public final class MetadataRepository {
configuration.getPlatformEventType().setRegistered(true);
typeLibrary.addType(configuration.getPlatformEventType());
if (jvm.isRecording()) {
settingsManager.setEventControl(configuration.getEventControl());
settingsManager.setEventControl(configuration.getEventControl(), true);
settingsManager.updateRetransform(Collections.singletonList((eventClass)));
}
setStaleMetadata();
@ -225,8 +225,8 @@ public final class MetadataRepository {
return configuration;
}
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() {

View File

@ -254,7 +254,7 @@ public final class PlatformRecorder {
currentChunk.setStartTime(startTime);
}
recording.setState(RecordingState.RUNNING);
updateSettings();
updateSettings(false);
recording.setStartTime(startTime);
writeMetaEvents();
} else {
@ -273,7 +273,7 @@ public final class PlatformRecorder {
startTime = Utils.epochNanosToInstant(startNanos);
recording.setStartTime(startTime);
recording.setState(RecordingState.RUNNING);
updateSettings();
updateSettings(false);
writeMetaEvents();
if (currentChunk != null) {
finishChunk(currentChunk, startTime, recording);
@ -337,7 +337,7 @@ public final class PlatformRecorder {
} else {
RepositoryChunk newChunk = null;
RequestEngine.doChunkEnd();
updateSettingsButIgnoreRecording(recording);
updateSettingsButIgnoreRecording(recording, false);
String path = null;
if (toDisk) {
@ -381,11 +381,11 @@ public final class PlatformRecorder {
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) {
@ -393,7 +393,7 @@ public final class PlatformRecorder {
list.add(r.getSettings());
}
}
MetadataRepository.getInstance().setSettings(list);
MetadataRepository.getInstance().setSettings(list, writeSettingEvents);
}

View File

@ -465,7 +465,7 @@ public final class PlatformRecording implements AutoCloseable {
synchronized (recorder) {
this.settings.put(id, value);
if (getState() == RecordingState.RUNNING) {
recorder.updateSettings();
recorder.updateSettings(true);
}
}
}
@ -486,7 +486,7 @@ public final class PlatformRecording implements AutoCloseable {
synchronized (recorder) {
this.settings = new LinkedHashMap<>(settings);
if (getState() == RecordingState.RUNNING && update) {
recorder.updateSettings();
recorder.updateSettings(true);
}
}
}

View File

@ -130,7 +130,7 @@ final class SettingsManager {
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 @@ final class SettingsManager {
eventControls.sort(Comparator.comparing(x -> x.getEventType().getName()));
}
for (EventControl ec : eventControls) {
setEventControl(ec);
setEventControl(ec, writeSettingEvents);
}
}
if (JVM.getJVM().getAllowedToDoEventRetransforms()) {
@ -211,7 +211,7 @@ final class SettingsManager {
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 @@ final class SettingsManager {
}
}
}
ec.writeActiveSettingEvent();
if (writeSettingEvents) {
ec.writeActiveSettingEvent();
}
if (shouldLog) {
Logger.log(LogTag.JFR_SETTING, LogLevel.INFO, "}");
}

View File

@ -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 @@ public final class TestActiveSettingEvent {
public static void main(String[] args) throws Throwable {
testDefaultSettings();
testProfileSettings();
testOnlyOnce();
testNewSettings();
testChangedSetting();
testUnregistered();
@ -74,6 +77,39 @@ public final class TestActiveSettingEvent {
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()) {