RFR: 8307526: [JFR] Better handling of tampered JFR repository [v6]

[Erik Gahlin]

On Fri, 30 Jun 2023 11:40:05 GMT, Joakim Nordström <jnordstrom at openjdk.org> wrote:

>> This change makes sure that "JFR Periodic Task" isn't interrupted by any errors. This can happen when an unfinished chunkfile is removed "in-flight", which would lead to the chunkfiles not being rotated properly, and the maxsize/maxage being ignored.
>> 
>> With this fix, when an unfinished chunkfile is detected, all of the chunkfiles in the recording are checked for existence (since one likely cause for this could be f.i. 'rm -r /tmp' being invoked, effectively deleting all chunkfiles). Upon seeing missing chunkfiles, an error is logged, and emitted to the recording to signal that some data might be missing.
>> A check to catch any missing chunkfiles is also added when dumping a recording -- this can happen if only finished chunkfiles have been removed, which is not detected in-flight.
>> 
>> Also, a check was added to the file purger to check whether the file had already been deleted to not fill the purge list with already removed files.
>
> Joakim Nordström has updated the pull request incrementally with three additional commits since the last revision:
> 
>  - Commit
>  - Commit
>  - Removed MissingChunkFileError

src/hotspot/share/jfr/jni/jfrJniMethod.cpp line 406:

> 404: 
> 405: JVM_ENTRY_NO_ENV(void, jfr_emit_data_loss(JNIEnv* env, jobject jvm, jlong bytes))
> 406:   EventDataLoss::commit(bytes, 0L);

If a value is missing, we use min_jlong to indicate not available or not applicable

src/jdk.jfr/share/classes/jdk/jfr/internal/PlatformRecorder.java line 525:

> 523:             } catch(Throwable t) {
> 524:                 // Catch everything and log, but don't allow it to end the periodic task
> 525:                 Logger.log(JFR_SYSTEM, ERROR, "Error in Periodic task: " + t.getClass().getName() + ", " + t.getMessage());

Not sure if it matters, but you could get an OOM when string are concatenated and the thread will end.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/14360#discussion_r1250856963
PR Review Comment: https://git.openjdk.org/jdk/pull/14360#discussion_r1250890601