infernap12/jdk
1
0
Fork 0
mirror of https://github.com/openjdk/jdk.git synced 2026-02-12 11:28:35 +00:00
Code Issues Packages Projects Releases Wiki Activity

8352066: JVM.commit() and JVM.flush() exhibit race conditions against JFR epochs

Browse Source 
Reviewed-by: egahlin
This commit is contained in:
Markus Grönlund 2025-03-17 10:47:18 +00:00
parent 0450ba9b65
commit d207ed3f7c
8 changed files with 23 additions and 56 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
src/hotspot/share/jfr/jni/jfrJniMethod.cpp
View File

@ -300,13 +300,13 @@ JVM_ENTRY_NO_ENV(jobject, jfr_new_event_writer(JNIEnv* env, jclass jvm))
return JfrJavaEventWriter::new_event_writer(thread);
JVM_END
NO_TRANSITION(void, jfr_event_writer_flush(JNIEnv* env, jclass jvm, jobject writer, jint used_size, jint requested_size))
JfrJavaEventWriter::flush(writer, used_size, requested_size, JavaThread::current());
NO_TRANSITION_END
JVM_ENTRY_NO_ENV(void, jfr_event_writer_flush(JNIEnv* env, jclass jvm, jobject writer, jint used_size, jint requested_size))
JfrJavaEventWriter::flush(writer, used_size, requested_size, thread);
JVM_END
NO_TRANSITION(jlong, jfr_commit(JNIEnv* env, jclass jvm, jlong next_position))
return JfrJavaEventWriter::commit(next_position);
NO_TRANSITION_END
JVM_ENTRY_NO_ENV(jlong, jfr_commit(JNIEnv* env, jclass jvm, jlong next_position))
return JfrJavaEventWriter::commit(next_position, thread);
JVM_END
JVM_ENTRY_NO_ENV(void, jfr_flush(JNIEnv* env, jclass jvm))
JfrRepository::flush(thread);

2
src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointManager.hpp
View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2016, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2016, 2025, 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

5
src/hotspot/share/jfr/recorder/service/jfrRecorderService.cpp
View File

@ -486,6 +486,7 @@ void JfrRecorderService::safepoint_clear() {
assert(SafepointSynchronize::is_at_safepoint(), "invariant");
_checkpoint_manager.begin_epoch_shift();
_storage.clear();
_checkpoint_manager.notify_threads();
_chunkwriter.set_time_stamp();
JfrDeprecationManager::on_safepoint_clear();
JfrStackTraceRepository::clear();
@ -650,9 +651,7 @@ size_t JfrRecorderService::flush() {
if (_string_pool.is_modified()) {
total_elements += flush_stringpool(_string_pool, _chunkwriter);
}
if (_stack_trace_repository.is_modified()) {
total_elements += flush_stacktrace(_stack_trace_repository, _chunkwriter);
}
total_elements += flush_stacktrace(_stack_trace_repository, _chunkwriter);
return flush_typeset(_checkpoint_manager, _chunkwriter) + total_elements;
}

6
src/hotspot/share/jfr/recorder/stacktrace/jfrStackTraceRepository.cpp
View File

@ -92,13 +92,9 @@ void JfrStackTraceRepository::destroy() {
_leak_profiler_instance = nullptr;
}
bool JfrStackTraceRepository::is_modified() const {
return _last_entries != _entries;
}
size_t JfrStackTraceRepository::write(JfrChunkWriter& sw, bool clear) {
MutexLocker lock(JfrStacktrace_lock, Mutex::_no_safepoint_check_flag);
if (_entries == 0) {
if ((_entries == _last_entries) && !clear) {
return 0;
}
int count = 0;

3
src/hotspot/share/jfr/recorder/stacktrace/jfrStackTraceRepository.hpp
View File

@ -1,5 +1,5 @@
/*
* Copyright (c) 2011, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2011, 2025, 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
@ -56,7 +56,6 @@ class JfrStackTraceRepository : public JfrCHeapObj {
static void destroy();
bool initialize();
bool is_modified() const;
static size_t clear();
static size_t clear(JfrStackTraceRepository& repo);
size_t write(JfrChunkWriter& cw, bool clear);

30
src/hotspot/share/jfr/support/jfrIntrinsics.cpp
View File

@ -37,35 +37,14 @@ static void assert_precondition(JavaThread* jt) {
DEBUG_ONLY(JfrJavaSupport::check_java_thread_in_java(jt);)
assert(jt->has_last_Java_frame(), "invariant");
}
static void assert_epoch_identity(JavaThread* jt, u2 current_epoch) {
assert_precondition(jt);
// Verify the epoch updates got written through also to the vthread object.
const u2 epoch_raw = ThreadIdAccess::epoch(jt->vthread());
const bool excluded = epoch_raw & excluded_bit;
assert(!excluded, "invariant");
assert(!JfrThreadLocal::is_excluded(jt), "invariant");
const u2 vthread_epoch = epoch_raw & epoch_mask;
assert(vthread_epoch == current_epoch, "invariant");
}
#endif // ASSERT
#endif
void* JfrIntrinsicSupport::write_checkpoint(JavaThread* jt) {
DEBUG_ONLY(assert_precondition(jt);)
assert(JfrThreadLocal::is_vthread(jt), "invariant");
const u2 vthread_thread_local_epoch = JfrThreadLocal::vthread_epoch(jt);
const u2 current_epoch = ThreadIdAccess::current_epoch();
MACOS_AARCH64_ONLY(ThreadWXEnable __wx(WXWrite, jt));
if (vthread_thread_local_epoch == current_epoch) {
// After the epoch test in the intrinsic, the thread sampler interleaved
// and suspended the thread. As part of taking a sample, it updated
// the vthread object and the thread local "for us". We are good.
DEBUG_ONLY(assert_epoch_identity(jt, current_epoch);)
ThreadInVMfromJava transition(jt);
return JfrJavaEventWriter::event_writer(jt);
}
const traceid vthread_tid = JfrThreadLocal::vthread_id(jt);
// Transition before reading the epoch generation anew, now as _thread_in_vm. Can safepoint here.
// Transition before reading the epoch generation, now as _thread_in_vm.
MACOS_AARCH64_ONLY(ThreadWXEnable __wx(WXWrite, jt));
ThreadInVMfromJava transition(jt);
JfrThreadLocal::set_vthread_epoch(jt, vthread_tid, ThreadIdAccess::current_epoch());
return JfrJavaEventWriter::event_writer(jt);
@ -74,12 +53,11 @@ void* JfrIntrinsicSupport::write_checkpoint(JavaThread* jt) {
void* JfrIntrinsicSupport::return_lease(JavaThread* jt) {
DEBUG_ONLY(assert_precondition(jt);)
MACOS_AARCH64_ONLY(ThreadWXEnable __wx(WXWrite, jt));
ThreadStateTransition::transition_from_java(jt, _thread_in_native);
ThreadInVMfromJava transition(jt);
assert(jt->jfr_thread_local()->has_java_event_writer(), "invariant");
assert(jt->jfr_thread_local()->shelved_buffer() != nullptr, "invariant");
JfrJavaEventWriter::flush(jt->jfr_thread_local()->java_event_writer(), 0, 0, jt);
assert(jt->jfr_thread_local()->shelved_buffer() == nullptr, "invariant");
ThreadStateTransition::transition_from_native(jt, _thread_in_Java);
return nullptr;
}

19
src/hotspot/share/jfr/writers/jfrJavaEventWriter.cpp
View File

@ -118,7 +118,7 @@ bool JfrJavaEventWriter::initialize() {
}
void JfrJavaEventWriter::flush(jobject writer, jint used, jint requested, JavaThread* jt) {
DEBUG_ONLY(JfrJavaSupport::check_java_thread_in_native(jt));
DEBUG_ONLY(JfrJavaSupport::check_java_thread_in_vm(jt));
assert(writer != nullptr, "invariant");
JfrBuffer* const current = jt->jfr_thread_local()->java_buffer();
assert(current != nullptr, "invariant");
@ -130,9 +130,6 @@ void JfrJavaEventWriter::flush(jobject writer, jint used, jint requested, JavaTh
const bool is_valid = buffer->free_size() >= (size_t)(used + requested);
u1* const new_current_position = is_valid ? buffer->pos() + used : buffer->pos();
assert(start_pos_offset != invalid_offset, "invariant");
// can safepoint here
MACOS_AARCH64_ONLY(ThreadWXEnable __wx(WXWrite, jt));
ThreadInVMfromNative transition(jt);
oop const w = JNIHandles::resolve_non_null(writer);
assert(w != nullptr, "invariant");
w->long_field_put(start_pos_offset, (jlong)buffer->pos());
@ -147,11 +144,10 @@ void JfrJavaEventWriter::flush(jobject writer, jint used, jint requested, JavaTh
}
}
jlong JfrJavaEventWriter::commit(jlong next_position) {
jlong JfrJavaEventWriter::commit(jlong next_position, JavaThread* jt) {
assert(next_position != 0, "invariant");
JavaThread* const jt = JavaThread::current();
assert(jt != nullptr, "invariant");
DEBUG_ONLY(JfrJavaSupport::check_java_thread_in_native(jt));
DEBUG_ONLY(JfrJavaSupport::check_java_thread_in_vm(jt));
JfrThreadLocal* const tl = jt->jfr_thread_local();
assert(tl != nullptr, "invariant");
assert(tl->has_java_event_writer(), "invariant");
@ -167,12 +163,11 @@ jlong JfrJavaEventWriter::commit(jlong next_position) {
}
// set_pos() has release semantics
current->set_pos(next);
if (!current->lease()) {
return next_position;
if (current->lease()) {
flush(tl->java_event_writer(), 0, 0, jt);
return 0; // signals that the buffer lease was returned.
}
assert(current->lease(), "invariant");
flush(tl->java_event_writer(), 0, 0, jt);
return 0; // signals that the buffer lease was returned.
return next_position;
}
class JfrJavaEventWriterNotificationClosure : public ThreadClosure {

2
src/hotspot/share/jfr/writers/jfrJavaEventWriter.hpp
View File

@ -50,7 +50,7 @@ class JfrJavaEventWriter : AllStatic {
static jobject event_writer(JavaThread* t);
static jobject new_event_writer(TRAPS);
static void flush(jobject writer, jint used, jint requested, JavaThread* jt);
static jlong commit(jlong next_position);
static jlong commit(jlong next_position, JavaThread* jt);
};
#endif // SHARE_JFR_WRITERS_JFRJAVAEVENTWRITER_HPP