8475: Writing events with fields not explicitly set can corrupt the recording

Author: jbachorik

core/org.openjdk.jmc.flightrecorder.writer/src/main/java/org/openjdk/jmc/flightrecorder/writer/Chunk.java

@@ -93,10 +93,6 @@ private void writeBuiltinType(LEB128Writer writer, TypedValueImpl typedValue) {
 			throw new IllegalArgumentException();
 		}
 
-		if (value == null && builtin != TypesImpl.Builtin.STRING) {
-			// skip the non-string built-in values
-			return;
-		}
 		switch (builtin) {
 		case STRING: {
 			if (value == null) {
@@ -114,35 +110,35 @@ private void writeBuiltinType(LEB128Writer writer, TypedValueImpl typedValue) {
 			break;
 		}
 		case BYTE: {
-			writer.writeByte((byte) value);
+			writer.writeByte(value == null ? (byte) 0 : (byte) value);
 			break;
 		}
 		case CHAR: {
-			writer.writeChar((char) value);
+			writer.writeChar(value == null ? (char) 0 : (char) value);
 			break;
 		}
 		case SHORT: {
-			writer.writeShort((short) value);
+			writer.writeShort(value == null ? (short) 0 : (short) value);
 			break;
 		}
 		case INT: {
-			writer.writeInt((int) value);
+			writer.writeInt(value == null ? 0 : (int) value);
 			break;
 		}
 		case LONG: {
-			writer.writeLong((long) value);
+			writer.writeLong(value == null ? 0L : (long) value);
 			break;
 		}
 		case FLOAT: {
-			writer.writeFloat((float) value);
+			writer.writeFloat(value == null ? 0.0f : (float) value);
 			break;
 		}
 		case DOUBLE: {
-			writer.writeDouble((double) value);
+			writer.writeDouble(value == null ? 0.0 : (double) value);
 			break;
 		}
 		case BOOLEAN: {
-			writer.writeBoolean((boolean) value);
+			writer.writeBoolean(value != null && (boolean) value);
 			break;
 		}
 		default: {

core/org.openjdk.jmc.flightrecorder.writer/src/main/java/org/openjdk/jmc/flightrecorder/writer/TypedValueImpl.java

@@ -33,6 +33,7 @@
  */
 package org.openjdk.jmc.flightrecorder.writer;
 
+import org.openjdk.jmc.flightrecorder.writer.api.Annotation;
 import org.openjdk.jmc.flightrecorder.writer.api.TypedValueBuilder;
 import org.openjdk.jmc.flightrecorder.writer.api.TypedValue;
 import org.openjdk.jmc.flightrecorder.writer.util.NonZeroHashCode;
@@ -142,13 +143,68 @@ public List<TypedFieldValueImpl> getFieldValues() {
 		for (TypedFieldImpl field : type.getFields()) {
 			TypedFieldValueImpl value = fields.get(field.getName());
 			if (value == null) {
-				value = new TypedFieldValueImpl(field, field.getType().nullValue());
+				value = new TypedFieldValueImpl(field, getDefaultImplicitFieldValue(field));
 			}
 			values.add(value);
 		}
 		return values;
 	}
 
+	/**
+	 * Gets the default value for a field when not explicitly provided by the user.
+	 * <p>
+	 * For event types (jdk.jfr.Event):
+	 * <ul>
+	 * <li>Fields annotated with {@code @Timestamp} receive {@link System#nanoTime()} as default,
+	 * providing a monotonic timestamp that will be >= the chunk's startTicks</li>
+	 * <li>Other fields receive null values</li>
+	 * </ul>
+	 * <p>
+	 * Note: JFR timestamps are stored as ticks relative to the chunk start, so the parser will
+	 * convert this absolute tick value to chunk-relative during reading.
+	 * <p>
+	 * <strong>Tick Frequency Assumption:</strong> This implementation assumes a 1:1 tick frequency
+	 * (1 tick = 1 nanosecond) as currently hardcoded in {@code RecordingImpl}. If the tick
+	 * frequency becomes configurable in the future, {@link System#nanoTime()} values will need to
+	 * be converted to ticks using: {@code nanoTime * ticksPerSecond / 1_000_000_000L}.
+	 *
+	 * @param field
+	 *            the field to get default value for
+	 * @return the default value for the field
+	 */
+	private TypedValueImpl getDefaultImplicitFieldValue(TypedFieldImpl field) {
+		if (!"jdk.jfr.Event".equals(type.getSupertype())) {
+			return field.getType().nullValue();
+		}
+
+		// Check if field is annotated with @Timestamp (any value means it's chunk-relative)
+		if (hasTimestampAnnotation(field)) {
+			// Use current nanoTime as default - will be valid and >= chunk startTicks
+			// NOTE: Assumes 1:1 tick frequency (1 tick = 1 ns) as per RecordingImpl line 280
+			return field.getType().asValue(System.nanoTime());
+		}
+
+		// For all other fields, return null value
+		// Null builtin values are handled properly by Chunk.writeBuiltinType()
+		return field.getType().nullValue();
+	}
+
+	/**
+	 * Checks if a field has the {@code @Timestamp} annotation.
+	 *
+	 * @param field
+	 *            the field to check
+	 * @return true if the field is annotated with @Timestamp
+	 */
+	private boolean hasTimestampAnnotation(TypedFieldImpl field) {
+		for (Annotation annotation : field.getAnnotations()) {
+			if ("jdk.jfr.Timestamp".equals(annotation.getType().getTypeName())) {
+				return true;
+			}
+		}
+		return false;
+	}
+
 	long getConstantPoolIndex() {
 		return cpIndex;
 	}

core/org.openjdk.jmc.flightrecorder.writer/src/main/java/org/openjdk/jmc/flightrecorder/writer/api/Type.java

@@ -201,6 +201,34 @@ public interface Type extends NamedType {
 	TypedValue asValue(Object value);
 
 	/**
+	 * Creates a typed null value for this type.
+	 * <p>
+	 * Use this method when you need to pass a null value for optional or missing complex-type
+	 * fields. Passing {@code null} directly to
+	 * {@link TypedValueBuilder#putField(String, TypedValue)} causes compilation ambiguity because
+	 * the method is overloaded with multiple parameter types.
+	 * <p>
+	 * For primitive types (int, long, String, etc.), you can pass primitive default/null values
+	 * directly. For complex types (Thread, StackTrace, custom types), use this method to create a
+	 * properly typed null value.
+	 * <p>
+	 * <strong>Example:</strong>
+	 *
+	 * <pre>
+	 * {
+	 * 	&#64;code
+	 * 	Types types = recording.getTypes();
+	 * 	Type stackTraceType = types.getType(Types.JDK.STACK_TRACE);
+	 * 	Type threadType = types.getType(Types.JDK.THREAD);
+	 *
+	 * 	Type eventType = recording.registerEventType("custom.Event");
+	 * 	recording.writeEvent(eventType.asValue(builder -> {
+	 * 		builder.putField("startTime", System.nanoTime()).putField("stackTrace", stackTraceType.nullValue()) // typed null
+	 * 				.putField("eventThread", threadType.nullValue()); // typed null
+	 * 	}));
+	 * }
+	 * </pre>
+	 *
 	 * @return a specific {@linkplain TypedValue} instance designated as the {@literal null} value
 	 *         for this type
 	 */

core/org.openjdk.jmc.flightrecorder.writer/src/main/java/org/openjdk/jmc/flightrecorder/writer/api/TypedValueBuilder.java

@@ -36,7 +36,40 @@
 import java.util.Map;
 import java.util.function.Consumer;
 
-/** A fluent API for lazy initialization of a composite type value */
+/**
+ * A fluent API for lazy initialization of a composite type value.
+ * <p>
+ * This builder provides a chainable interface for setting field values in complex types. Use it
+ * with {@link Type#asValue(java.util.function.Consumer)} to construct typed values.
+ * <h2>Handling Null Values</h2>
+ * <p>
+ * When setting field values, avoid passing {@code null} directly as it causes compilation ambiguity
+ * due to overloaded methods. Instead:
+ * <ul>
+ * <li>For primitive types (String, int, long, etc.): cast to the specific type, e.g.,
+ * {@code (String) null}</li>
+ * <li>For complex types (Thread, StackTrace, custom types): use {@link Type#nullValue()}</li>
+ * </ul>
+ * <p>
+ * <strong>Example:</strong>
+ *
+ * <pre>
+ * {
+ * 	&#64;code
+ * 	Types types = recording.getTypes();
+ * 	Type threadType = types.getType(Types.JDK.THREAD);
+ *
+ * 	Type eventType = recording.registerEventType("custom.Event", builder -> {
+ * 		builder.addField("message", Types.Builtin.STRING).addField("thread", Types.JDK.THREAD);
+ * 	});
+ *
+ * 	recording.writeEvent(eventType.asValue(builder -> {
+ * 		builder.putField("message", (String) null) // primitive null with cast
+ * 				.putField("thread", threadType.nullValue()); // complex type null
+ * 	}));
+ * }
+ * </pre>
+ */
 public interface TypedValueBuilder {
 	Type getType();