From 642112399b552d4687af16ed156ece926c21b5c1 Mon Sep 17 00:00:00 2001 From: egahlin Date: Wed, 12 Dec 2018 18:35:26 +0100 Subject: [PATCH] 8165675: Trace event for thread park has incorrect unit for timeout Reviewed-by: mgronlun --- .../jdk/jfr/consumer/RecordedObject.java | 12 ++++- .../jdk/jfr/internal/tool/PrettyWriter.java | 43 ++++++++++++++-- .../event/runtime/TestThreadParkEvent.java | 49 +++++++++++++++---- .../jmx/security/TestEnoughPermission.java | 2 +- test/lib/jdk/test/lib/jfr/Events.java | 47 ++++++++++++++++++ 5 files changed, 138 insertions(+), 15 deletions(-) diff --git a/src/share/classes/jdk/jfr/consumer/RecordedObject.java b/src/share/classes/jdk/jfr/consumer/RecordedObject.java index 2d9ed1be6..5633c6068 100644 --- a/src/share/classes/jdk/jfr/consumer/RecordedObject.java +++ b/src/share/classes/jdk/jfr/consumer/RecordedObject.java @@ -726,6 +726,9 @@ public class RecordedObject { private Duration getDuration(long timespan, String name) throws InternalError { ValueDescriptor v = getValueDescriptor(descriptors, name, null); + if (timespan == Long.MIN_VALUE) { + return Duration.ofSeconds(Long.MIN_VALUE, 0); + } Timespan ts = v.getAnnotation(Timespan.class); if (ts != null) { switch (ts.value()) { @@ -797,13 +800,16 @@ public class RecordedObject { return getInstant(Short.toUnsignedLong((Byte) u), name); } } - throw newIllegalArgumentException(name, "java,time.Instant"); + throw newIllegalArgumentException(name, "java.time.Instant"); } private Instant getInstant(long timestamp, String name) { ValueDescriptor v = getValueDescriptor(descriptors, name, null); Timestamp ts = v.getAnnotation(Timestamp.class); if (ts != null) { + if (timestamp == Long.MIN_VALUE) { + return Instant.MIN; + } switch (ts.value()) { case Timestamp.MILLISECONDS_SINCE_EPOCH: return Instant.ofEpochMilli(timestamp); @@ -884,6 +890,10 @@ public class RecordedObject { // package private for now. Used by EventWriter OffsetDateTime getOffsetDateTime(String name) { + Instant instant = getInstant(name); + if (instant.equals(Instant.MIN)) { + return OffsetDateTime.MIN; + } return OffsetDateTime.ofInstant(getInstant(name), timeConverter.getZoneOffset()); } diff --git a/src/share/classes/jdk/jfr/internal/tool/PrettyWriter.java b/src/share/classes/jdk/jfr/internal/tool/PrettyWriter.java index 199622ae4..bb1de603a 100644 --- a/src/share/classes/jdk/jfr/internal/tool/PrettyWriter.java +++ b/src/share/classes/jdk/jfr/internal/tool/PrettyWriter.java @@ -281,7 +281,7 @@ public final class PrettyWriter extends EventPrintWriter { private void printValue(Object value, ValueDescriptor field, String postFix) { if (value == null) { - println("null" + postFix); + println("N/A" + postFix); return; } if (value instanceof RecordedObject) { @@ -320,6 +320,35 @@ public final class PrettyWriter extends EventPrintWriter { return; } } + if (value instanceof Double) { + Double d = (Double) value; + if (Double.isNaN(d) || d == Double.NEGATIVE_INFINITY) { + println("N/A"); + return; + } + } + if (value instanceof Float) { + Float f = (Float) value; + if (Float.isNaN(f) || f == Float.NEGATIVE_INFINITY) { + println("N/A"); + return; + } + } + if (value instanceof Long) { + Long l = (Long) value; + if (l == Long.MIN_VALUE) { + println("N/A"); + return; + } + } + if (value instanceof Integer) { + Integer i = (Integer) value; + if (i == Integer.MIN_VALUE) { + println("N/A"); + return; + } + } + String text = String.valueOf(value); if (value instanceof String) { text = "\"" + text + "\""; @@ -443,6 +472,10 @@ public final class PrettyWriter extends EventPrintWriter { private boolean printFormatted(ValueDescriptor field, Object value) { if (value instanceof Duration) { Duration d = (Duration) value; + if (d.getSeconds() == Long.MIN_VALUE) { + println("N/A"); + return true; + } double s = d.getNano() / 1000_000_000.0 + (int) (d.getSeconds() % 60); if (s < 1.0) { if (s < 0.001) { @@ -460,8 +493,12 @@ public final class PrettyWriter extends EventPrintWriter { return true; } if (value instanceof OffsetDateTime) { - OffsetDateTime zdt = (OffsetDateTime) value; - println(TIME_FORMAT.format(zdt)); + OffsetDateTime odt = (OffsetDateTime) value; + if (odt.equals(OffsetDateTime.MIN)) { + println("N/A"); + return true; + } + println(TIME_FORMAT.format(odt)); return true; } Percentage percentage = field.getAnnotation(Percentage.class); diff --git a/test/jdk/jfr/event/runtime/TestThreadParkEvent.java b/test/jdk/jfr/event/runtime/TestThreadParkEvent.java index 3b9003148..06c6b78bb 100644 --- a/test/jdk/jfr/event/runtime/TestThreadParkEvent.java +++ b/test/jdk/jfr/event/runtime/TestThreadParkEvent.java @@ -25,22 +25,25 @@ package jdk.jfr.event.runtime; -import static jdk.test.lib.Asserts.assertFalse; import static jdk.test.lib.Asserts.assertTrue; +import java.nio.file.Files; +import java.nio.file.Paths; import java.time.Duration; +import java.time.Instant; import java.util.List; import java.util.concurrent.CountDownLatch; import java.util.concurrent.locks.LockSupport; +import java.util.function.Consumer; import jdk.jfr.Recording; import jdk.jfr.consumer.RecordedEvent; +import jdk.test.lib.Asserts; import jdk.test.lib.jfr.EventNames; import jdk.test.lib.jfr.Events; import jdk.test.lib.management.ThreadMXBeanTool; import jdk.test.lib.thread.TestThread; - /** * @test * @key jfr @@ -58,12 +61,39 @@ public class TestThreadParkEvent { } public static void main(String[] args) throws Throwable { + testParkNoTimeout(); + testParkTimeout(); + testParkUntil(); + } + + private static void testParkNoTimeout() throws Exception { + RecordedEvent event = testPark(x -> LockSupport.park(x), Thread.State.WAITING); + Events.assertMissingValue(event, "timeout"); + Events.assertMissingValue(event, "until"); + } + + private static void testParkTimeout() throws Exception { + Duration expected = Duration.ofNanos(1_234_567_890_123L); + RecordedEvent event = testPark(x -> LockSupport.parkNanos(x, expected.toNanos()), Thread.State.TIMED_WAITING); + Events.assertDuration(event, "timeout", expected); + Events.assertMissingValue(event, "until"); + } + + private static void testParkUntil() throws Exception { + long epochMillis = Instant.now().plusSeconds(1000000).toEpochMilli(); + RecordedEvent event = testPark(x -> LockSupport.parkUntil(x, epochMillis), Thread.State.TIMED_WAITING); + Events.assertMissingValue(event, "timeout"); + Events.assertInstant(event, "until", Instant.ofEpochMilli(epochMillis)); + } + + static RecordedEvent testPark(Consumer parkOperation, Thread.State threadState) throws Exception { + final CountDownLatch stop = new CountDownLatch(1); final Blocker blocker = new Blocker(); TestThread parkThread = new TestThread(new Runnable() { public void run() { while (stop.getCount() > 0) { - LockSupport.park(blocker); + parkOperation.accept(blocker); } } }); @@ -73,7 +103,7 @@ public class TestThreadParkEvent { try { recording.start(); parkThread.start(); - ThreadMXBeanTool.waitUntilBlockingOnObject(parkThread, Thread.State.WAITING, blocker); + ThreadMXBeanTool.waitUntilBlockingOnObject(parkThread, threadState, blocker); // sleep so we know the event is recorded Thread.sleep(2 * THRESHOLD_MILLIS); } finally { @@ -82,22 +112,21 @@ public class TestThreadParkEvent { parkThread.join(); recording.stop(); } - List events = Events.fromRecording(recording); Events.hasEvents(events); - boolean isAnyFound = false; + RecordedEvent foundEvent = null; for (RecordedEvent event : events) { System.out.println("Event:" + event); String klassName = Events.assertField(event, "parkedClass.name").notNull().getValue(); if (klassName.equals(blocker.getClass().getName().replace('.', '/'))) { - assertFalse(isAnyFound, "Found more than 1 event"); - isAnyFound = true; - Events.assertField(event, "timeout").equal(0L); + Asserts.assertNull(foundEvent , "Found more than 1 event"); Events.assertField(event, "address").notEqual(0L); Events.assertEventThread(event, parkThread); + foundEvent = event; } } - assertTrue(isAnyFound, "Correct event not found"); + Asserts.assertNotNull(foundEvent, "Correct event not found"); + return foundEvent; } } diff --git a/test/jdk/jfr/jmx/security/TestEnoughPermission.java b/test/jdk/jfr/jmx/security/TestEnoughPermission.java index 3c406debe..5fc80b545 100644 --- a/test/jdk/jfr/jmx/security/TestEnoughPermission.java +++ b/test/jdk/jfr/jmx/security/TestEnoughPermission.java @@ -85,7 +85,7 @@ public class TestEnoughPermission { System.out.println("E"); bean.stopRecording(recId); - final Path path = Paths.get(".", String.format("rec%d.jfr", recId)); + final Path path = Paths.get(".", "rec" + recId + ".jfr"); bean.copyTo(recId, path.toString()); //EventSet events = EventSet.fromFile(path); return recId; diff --git a/test/lib/jdk/test/lib/jfr/Events.java b/test/lib/jdk/test/lib/jfr/Events.java index cbcca1a2f..b30ac3d62 100644 --- a/test/lib/jdk/test/lib/jfr/Events.java +++ b/test/lib/jdk/test/lib/jfr/Events.java @@ -33,6 +33,8 @@ import static jdk.test.lib.Asserts.fail; import java.io.File; import java.io.IOException; import java.nio.file.Path; +import java.time.Duration; +import java.time.Instant; import java.util.List; import java.lang.management.ManagementFactory; @@ -40,6 +42,8 @@ import jdk.jfr.AnnotationElement; import jdk.jfr.EventType; import jdk.jfr.Recording; import jdk.jfr.SettingDescriptor; +import jdk.jfr.Timespan; +import jdk.jfr.Timestamp; import jdk.jfr.ValueDescriptor; import jdk.jfr.consumer.RecordingFile; import jdk.test.lib.Asserts; @@ -184,6 +188,49 @@ public class Events { assertThread(assertField(event, structName).notNull().getValue(), thread); } + public static void assertDuration(RecordedEvent event, String name, Duration duration) { + assertEquals(event.getDuration(name), duration); + } + + public static void assertInstant(RecordedEvent event, String name, Instant instant) { + assertEquals(event.getInstant(name), instant); + } + + public static void assertMissingValue(RecordedEvent event, String name) { + ValueDescriptor v = event.getEventType().getField(name); + if (v.getAnnotation(Timespan.class) != null) { + Duration d = event.getDuration(name); + assertTrue(d.getSeconds() == Long.MIN_VALUE && d.getNano() == 0); + return; + } + if (v.getAnnotation(Timestamp.class) != null) { + Instant instant = event.getInstant(name); + assertTrue(instant.equals(Instant.MIN)); + return; + } + if (v.getTypeName().equals("double")) { + double d = event.getDouble(name); + assertTrue(Double.isNaN(d) || d == Double.NEGATIVE_INFINITY); + return; + } + if (v.getTypeName().equals("float")) { + float f = event.getFloat(name); + assertTrue(Float.isNaN(f) || f == Float.NEGATIVE_INFINITY); + return; + } + if (v.getTypeName().equals("int")) { + int i = event.getInt(name); + assertTrue(i == Integer.MIN_VALUE); + return; + } + if (v.getTypeName().equals("long")) { + assertEquals(event.getLong(name), Long.MIN_VALUE); + return; + } + Object o = event.getValue(name); + Asserts.assertNull(o); + } + private static void assertThread(RecordedThread eventThread, Thread thread) { assertNotNull(eventThread, "Thread in event was null"); assertEquals(eventThread.getJavaThreadId(), thread.getId(), "Wrong thread id"); -- GitLab