Skip to content

Commit

Permalink
8293864: Kitchensink24HStress.java fails with SIGSEGV in JfrCheckpoin…
Browse files Browse the repository at this point in the history
…tManager::lease

Reviewed-by: egahlin
  • Loading branch information
Markus Grönlund committed Oct 10, 2022
1 parent c5f462e commit 35d17a0
Show file tree
Hide file tree
Showing 9 changed files with 173 additions and 117 deletions.
227 changes: 138 additions & 89 deletions src/hotspot/share/jfr/recorder/checkpoint/jfrCheckpointManager.cpp

Large diffs are not rendered by default.

Expand Up @@ -57,9 +57,11 @@ class JfrCheckpointManager : public JfrCHeapObj {
public:
typedef JfrCheckpointMspace::Node Buffer;
typedef JfrCheckpointMspace::NodePtr BufferPtr;
typedef const JfrCheckpointMspace::Node* ConstBufferPtr;
private:
JfrCheckpointMspace* _global_mspace;
JfrThreadLocalCheckpointMspace* _thread_local_mspace;
JfrThreadLocalCheckpointMspace* _virtual_thread_local_mspace;
JfrChunkWriter& _chunkwriter;

JfrCheckpointManager(JfrChunkWriter& cw);
Expand All @@ -69,14 +71,16 @@ class JfrCheckpointManager : public JfrCHeapObj {
bool initialize();
static void destroy();

static BufferPtr get_thread_local(Thread* thread);
static void set_thread_local(Thread* thread, BufferPtr buffer);
static BufferPtr acquire_thread_local(size_t size, Thread* thread);
static BufferPtr get_virtual_thread_local(Thread* thread);
static void set_virtual_thread_local(Thread* thread, BufferPtr buffer);
static BufferPtr acquire_virtual_thread_local(Thread* thread, size_t size);
static BufferPtr new_virtual_thread_local(Thread* thread, size_t size = 0);

static BufferPtr lease(Thread* thread, bool previous_epoch = false, size_t size = 0);
static BufferPtr lease(BufferPtr old, Thread* thread, size_t size);
static BufferPtr lease_thread_local(Thread* thread, size_t size = 0);
static BufferPtr lease_global(Thread* thread, bool previous_epoch = false, size_t size = 0);

static BufferPtr acquire(Thread* thread, JfrCheckpointBufferKind kind = JFR_THREADLOCAL, bool previous_epoch = false, size_t size = 0);
static BufferPtr renew(ConstBufferPtr old, Thread* thread, size_t size, JfrCheckpointBufferKind kind = JFR_THREADLOCAL);
static BufferPtr flush(BufferPtr old, size_t used, size_t requested, Thread* thread);

size_t clear();
Expand Down
Expand Up @@ -27,26 +27,27 @@
#include "jfr/recorder/checkpoint/jfrCheckpointWriter.hpp"
#include "jfr/utilities/jfrBlob.hpp"
#include "jfr/writers/jfrBigEndianWriter.hpp"
#include "runtime/thread.inline.hpp"

JfrCheckpointFlush::JfrCheckpointFlush(Type* old, size_t used, size_t requested, Thread* t) :
_result(JfrCheckpointManager::flush(old, used, requested, t)) {}

JfrCheckpointWriter::JfrCheckpointWriter(JfrCheckpointType type /* GENERIC */) :
JfrCheckpointWriterBase(JfrCheckpointManager::lease(Thread::current()), Thread::current()),
JfrCheckpointWriter::JfrCheckpointWriter(bool header /* true */, JfrCheckpointType type /* GENERIC */, JfrCheckpointBufferKind kind /* JFR_GLOBAL */) :
JfrCheckpointWriterBase(JfrCheckpointManager::acquire(Thread::current(), kind), Thread::current()),
_time(JfrTicks::now()),
_offset(0),
_count(0),
_type(type),
_header(true) {
_header(header) {
assert(this->is_acquired(), "invariant");
assert(0 == this->current_offset(), "invariant");
if (_header) {
reserve(sizeof(JfrCheckpointEntry));
}
}

JfrCheckpointWriter::JfrCheckpointWriter(Thread* thread, bool header /* true */, JfrCheckpointType type /* GENERIC */, bool global_lease /* true */) :
JfrCheckpointWriterBase(global_lease ? JfrCheckpointManager::lease(thread) : JfrCheckpointManager::lease_thread_local(thread), thread),
JfrCheckpointWriter::JfrCheckpointWriter(Thread* thread, bool header /* true */, JfrCheckpointType type /* GENERIC */, JfrCheckpointBufferKind kind /* JFR_GLOBAL */) :
JfrCheckpointWriterBase(JfrCheckpointManager::acquire(thread, kind), thread),
_time(JfrTicks::now()),
_offset(0),
_count(0),
Expand All @@ -60,7 +61,7 @@ JfrCheckpointWriter::JfrCheckpointWriter(Thread* thread, bool header /* true */,
}

JfrCheckpointWriter::JfrCheckpointWriter(bool previous_epoch, Thread* thread, JfrCheckpointType type /* GENERIC */) :
JfrCheckpointWriterBase(JfrCheckpointManager::lease(thread, previous_epoch), thread),
JfrCheckpointWriterBase(JfrCheckpointManager::lease_global(thread, previous_epoch), thread),
_time(JfrTicks::now()),
_offset(0),
_count(0),
Expand Down
Expand Up @@ -70,9 +70,9 @@ class JfrCheckpointWriter : public JfrCheckpointWriterBase {
const u1* session_data(size_t* size, bool move = false, const JfrCheckpointContext* ctx = NULL);
void release();
JfrCheckpointWriter(bool previous_epoch, Thread* thread, JfrCheckpointType type = GENERIC);
public:
JfrCheckpointWriter(JfrCheckpointType type = GENERIC);
JfrCheckpointWriter(Thread* thread, bool header = true, JfrCheckpointType mode = GENERIC, bool global_lease = true);
public:
JfrCheckpointWriter(bool header = true, JfrCheckpointType mode = GENERIC, JfrCheckpointBufferKind kind = JFR_GLOBAL);
JfrCheckpointWriter(Thread* thread, bool header = true, JfrCheckpointType mode = GENERIC, JfrCheckpointBufferKind kind = JFR_GLOBAL);
~JfrCheckpointWriter();
void write_type(JfrTypeId type_id);
void write_count(u4 nof_entries);
Expand Down
Expand Up @@ -36,6 +36,7 @@
#include "memory/resourceArea.hpp"
#include "runtime/javaThread.hpp"
#include "runtime/semaphore.hpp"
#include "runtime/thread.inline.hpp"
#include "utilities/macros.hpp"

class JfrSerializerRegistration : public JfrCHeapObj {
Expand Down Expand Up @@ -104,7 +105,7 @@ void JfrTypeManager::write_threads(JfrCheckpointWriter& writer) {
JfrBlobHandle JfrTypeManager::create_thread_blob(JavaThread* jt, traceid tid /* 0 */, oop vthread /* nullptr */) {
assert(jt != NULL, "invariant");
ResourceMark rm(jt);
JfrCheckpointWriter writer(jt, true, THREADS, false); // Thread local lease for blob creation.
JfrCheckpointWriter writer(jt, true, THREADS, JFR_THREADLOCAL); // Thread local lease for blob creation.
// TYPE_THREAD and count is written unconditionally for blobs, also for vthreads.
writer.write_type(TYPE_THREAD);
writer.write_count(1);
Expand All @@ -119,7 +120,7 @@ void JfrTypeManager::write_checkpoint(Thread* t, traceid tid /* 0 */, oop vthrea
assert(current != NULL, "invariant");
const bool is_vthread = vthread != nullptr;
ResourceMark rm(current);
JfrCheckpointWriter writer(current, true, THREADS, !is_vthread); // Virtual Threads use thread local lease.
JfrCheckpointWriter writer(current, true, THREADS, is_vthread ? JFR_VIRTUAL_THREADLOCAL : JFR_THREADLOCAL);
if (is_vthread) {
// TYPE_THREAD and count is written later as part of vthread bulk serialization.
writer.set_count(1); // Only a logical marker for the checkpoint header.
Expand Down Expand Up @@ -202,7 +203,7 @@ static bool register_static_type(JfrTypeId id, bool permit_cache, JfrSerializer*
assert(!types.in_list(registration), "invariant");
DEBUG_ONLY(assert_not_registered_twice(id, types);)
if (JfrRecorder::is_recording()) {
JfrCheckpointWriter writer(STATICS);
JfrCheckpointWriter writer(Thread::current(), true, STATICS);
registration->invoke(writer);
}
types.add(registration);
Expand Down
8 changes: 3 additions & 5 deletions src/hotspot/share/jfr/recorder/storage/jfrBuffer.cpp
Expand Up @@ -37,7 +37,7 @@ JfrBuffer::JfrBuffer() : _next(NULL),
_flags(0),
_context(0) {}

bool JfrBuffer::initialize(size_t header_size, size_t size) {
void JfrBuffer::initialize(size_t header_size, size_t size) {
assert(_next == NULL, "invariant");
assert(_identity == NULL, "invariant");
_header_size = (u2)header_size;
Expand All @@ -48,10 +48,9 @@ bool JfrBuffer::initialize(size_t header_size, size_t size) {
assert(!transient(), "invariant");
assert(!lease(), "invariant");
assert(!retired(), "invariant");
return true;
}

void JfrBuffer::reinitialize(bool exclusion /* false */) {
void JfrBuffer::reinitialize() {
acquire_critical_section_top();
set_pos(start());
release_critical_section_top(start());
Expand Down Expand Up @@ -173,8 +172,7 @@ size_t JfrBuffer::unflushed_size() const {
enum FLAG {
RETIRED = 1,
TRANSIENT = 2,
LEASE = 4,
EXCLUDED = 8
LEASE = 4
};

inline u1 load(const volatile u1* dest) {
Expand Down
4 changes: 2 additions & 2 deletions src/hotspot/share/jfr/recorder/storage/jfrBuffer.hpp
Expand Up @@ -79,8 +79,8 @@ class JfrBuffer {

public:
JfrBuffer();
bool initialize(size_t header_size, size_t size);
void reinitialize(bool exclusion = false);
void initialize(size_t header_size, size_t size);
void reinitialize();

const u1* start() const {
return ((const u1*)this) + _header_size;
Expand Down
Expand Up @@ -218,10 +218,7 @@ inline typename FreeListType::NodePtr JfrMemorySpace<Client, RetrievalPolicy, Fr
}
NodePtr node = new (allocation) Node();
assert(node != NULL, "invariant");
if (!node->initialize(sizeof(Node), aligned_size_bytes)) {
JfrCHeapObj::free(node, aligned_size_bytes + sizeof(Node));
return NULL;
}
node->initialize(sizeof(Node), aligned_size_bytes);
return node;
}

Expand Down
6 changes: 6 additions & 0 deletions src/hotspot/share/jfr/utilities/jfrTypes.hpp
Expand Up @@ -57,4 +57,10 @@ enum JfrCheckpointType {
THREADS = 8
};

enum JfrCheckpointBufferKind {
JFR_GLOBAL,
JFR_THREADLOCAL,
JFR_VIRTUAL_THREADLOCAL
};

#endif // SHARE_JFR_UTILITIES_JFRTYPES_HPP

1 comment on commit 35d17a0

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sign in to comment.