Skip to content
This repository has been archived by the owner. It is now read-only.
Permalink
Browse files
8259036: Failed JfrVersionSystem invariant when VM built with -fno-el…
…ide-constructors

Reviewed-by: egahlin
  • Loading branch information
Markus Grönlund committed Jan 19, 2021
1 parent c0e9c44 commit 5cfb36e79ad59ed057ba12dbae674cd79f77b060
Show file tree
Hide file tree
Showing 5 changed files with 117 additions and 141 deletions.
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2020, 2021 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
@@ -47,7 +47,7 @@ inline Node* mark_for_removal(Node* node) {

/*
* The insertion marker (i.e. the insertion bit) is represented by '[ ]' as part of state description comments:
* "node --> next" becomes "[node] --> next", in an attempt to convey node as being exlusively reserved.
* "node --> next" becomes "[node] --> next", in an attempt to convey the node as exlusively reserved.
*/
template <typename Node>
inline bool mark_for_insertion(Node* node, const Node* tail) {
@@ -66,8 +66,7 @@ Node* find_adjacent(Node* head, const Node* tail, Node** predecessor, VersionHan
Node* predecessor_next = NULL;
while (true) {
Node* current = head;
version_handle.checkout();
assert(version_handle.is_tracked(), "invariant");
version_handle->checkout();
Node* next = Atomic::load_acquire(&current->_next);
do {
assert(next != NULL, "invariant");
@@ -117,7 +116,6 @@ void JfrConcurrentLinkedListHost<Client, SearchPolicy, AllocPolicy>::insert_head
while (true) {
// Find an adjacent predecessor and successor node pair.
successor = find_adjacent<Node, VersionHandle, HeadNode>(head, tail, &predecessor, version_handle, predicate);
assert(version_handle.is_tracked(), "invariant");
// Invariant (adjacency): predecessor --> successor
// Invariant (optional: key-based total order): predecessor->key() < key && key <= successor->key().
// We can now attempt to insert the new node in-between.
@@ -149,7 +147,6 @@ void JfrConcurrentLinkedListHost<Client, SearchPolicy, AllocPolicy>::insert_tail
while (true) {
// Find an adjacent predecessor and successor node pair, where the successor == tail
const NodePtr successor = find_adjacent<Node, VersionHandle, LastNode>(last, tail, &predecessor, version_handle, predicate);
assert(version_handle.is_tracked(), "invariant");
assert(successor == tail, "invariant");
// Invariant: predecessor --> successor
// We first attempt to mark the predecessor node to signal our intent of performing an insertion.
@@ -182,7 +179,7 @@ void JfrConcurrentLinkedListHost<Client, SearchPolicy, AllocPolicy>::insert_tail
head->_next = node;
// Invariant: head --> [node] --> tail
}
version_handle.release(); // release_store_fence
OrderAccess::storestore();
// Publish the inserted node by removing the insertion marker.
node->_next = const_cast<NodePtr>(tail);
// Invariant: last --> node --> tail (possibly also head --> node --> tail)
@@ -204,7 +201,6 @@ typename Client::Node* JfrConcurrentLinkedListHost<Client, SearchPolicy, AllocPo
while (true) {
// Find an adjacent predecessor and successor node pair.
successor = find_adjacent<Node, VersionHandle, SearchPolicy>(head, tail, &predecessor, version_handle, predicate);
assert(version_handle.is_tracked(), "invariant");
if (successor == tail) {
return NULL;
}
@@ -228,7 +224,6 @@ typename Client::Node* JfrConcurrentLinkedListHost<Client, SearchPolicy, AllocPo
// Physically excise using slow path, can be completed asynchronously by other threads.
Identity<Node> excise(successor);
find_adjacent<Node, VersionHandle, Identity>(head, tail, &predecessor, version_handle, excise);
assert(version_handle.is_tracked(), "invariant");
}
if (last != NULL && Atomic::load_acquire(&last->_next) == successor) {
guarantee(!insert_is_head, "invariant");
@@ -237,11 +232,9 @@ typename Client::Node* JfrConcurrentLinkedListHost<Client, SearchPolicy, AllocPo
find_adjacent<Node, VersionHandle, LastNode>(last, tail, &predecessor, version_handle, excise);
// Invariant: successor excised from last list
}
// Increment the current version so we can track when other threads have seen this update.
VersionType version = version_handle.increment();
version_handle.release(); // release_store_fence
// Rendezvous with checkouts for versions less than this version.
version_handle.await(version);
// Commit the modification back to the version control system.
// It blocks until all checkouts for versions earlier than the commit have been released.
version_handle->commit();
// At this point we know there can be no references onto the excised node. It is safe, enjoy it.
return successor;
}
@@ -255,8 +248,7 @@ bool JfrConcurrentLinkedListHost<Client, SearchPolicy, AllocPolicy>::in_list(con
assert(head != tail, "invariant");
VersionHandle version_handle = _client->get_version_handle();
const Node* current = head;
version_handle.checkout();
assert(version_handle.is_tracked(), "invariant");
version_handle->checkout();
const Node* next = Atomic::load_acquire(&current->_next);
while (true) {
if (!is_marked_for_removal(next)) {
@@ -281,8 +273,7 @@ inline void JfrConcurrentLinkedListHost<Client, SearchPolicy, AllocPolicy>::iter
assert(head != tail, "invariant");
VersionHandle version_handle = _client->get_version_handle();
NodePtr current = head;
version_handle.checkout();
assert(version_handle.is_tracked(), "invariant");
version_handle->checkout();
NodePtr next = Atomic::load_acquire(&current->_next);
while (true) {
if (!is_marked_for_removal(next)) {
@@ -70,7 +70,7 @@ void JfrConcurrentQueue<NodeType, AllocPolicy>::iterate(Callback& cb) {

template <typename NodeType, typename AllocPolicy>
inline JfrVersionSystem::Handle JfrConcurrentQueue<NodeType, AllocPolicy>::get_version_handle() {
return _version_system.get_handle();
return _version_system.get();
}

template <typename NodeType, typename AllocPolicy>
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2017, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2017, 2021, 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
@@ -30,8 +30,6 @@

template <typename T>
class RefCountHandle {
template <typename, typename>
friend class RefCountPointer;
private:
const T* _ptr;

@@ -51,9 +49,8 @@ class RefCountHandle {

~RefCountHandle() {
if (_ptr != NULL) {
const T* temp = _ptr;
_ptr->remove_ref();
_ptr = NULL;
temp->remove_ref();
}
}

@@ -76,18 +73,41 @@ class RefCountHandle {
return _ptr != NULL;
}

const T & operator->() const {
const T& operator->() const {
return *_ptr;
}

T& operator->() {
return *const_cast<T*>(_ptr);
}

static RefCountHandle<T> make(const T* ptr) {
return ptr;
}
};

class SingleThreadedRefCounter {
private:
mutable intptr_t _refs;
public:
SingleThreadedRefCounter() : _refs(0) {}

void inc() const {
++_refs;
}

bool dec() const {
return --_refs == 0;
}

intptr_t current() const {
return _refs;
}
};

class MultiThreadedRefCounter {
private:
mutable volatile int _refs;
mutable volatile intptr_t _refs;
public:
MultiThreadedRefCounter() : _refs(0) {}

@@ -99,7 +119,7 @@ class MultiThreadedRefCounter {
return 0 == Atomic::add(&_refs, (-1));
}

int current() const {
intptr_t current() const {
return _refs;
}
};
@@ -146,8 +166,7 @@ class RefCountPointer : public JfrCHeapObj {
}

static RefHandle make(const T* ptr) {
assert(ptr != NULL, "invariant");
return RefHandle(new RefCountPointer<T, RefCountImpl>(ptr));
return RefHandle::make(new RefCountPointer<T, RefCountImpl>(ptr));
}
};

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

#include "jfr/utilities/jfrAllocation.hpp"
#include "jfr/utilities/jfrRefCountPointer.hpp"
#include "jfr/utilities/jfrTypes.hpp"
#include "memory/padded.hpp"

@@ -66,58 +67,50 @@ class JfrVersionSystem : public JfrCHeapObj {
typedef traceid Type;
private:
class Node : public JfrCHeapObj {
public:
friend class JfrVersionSystem;
template <typename>
friend class RefCountHandle;
private:
JfrVersionSystem* const _system;
Node* _next;
Type _version;
bool _live;
Node();
mutable Type _version;
SingleThreadedRefCounter _ref_counter;
mutable bool _live;
Node(JfrVersionSystem* system);
void add_ref() const;
void remove_ref() const;
Type version() const;
void set(Type version);
};
typedef Node* NodePtr;
public:
class Handle {
private:
JfrVersionSystem* _system;
NodePtr _node;
Handle(JfrVersionSystem* system);
void set(Type version) const;
public:
Handle();
~Handle();
void checkout();
void release();
Type increment();
void await(Type version);
DEBUG_ONLY(bool is_tracked() const;)
friend class JfrVersionSystem;
void commit();
const Node* operator->() const { return this; }
Node* operator->() { return this; }
};
typedef Node* NodePtr;

public:
JfrVersionSystem();
~JfrVersionSystem();
void reset();

// to access the versioning system
Handle get_handle();
Handle checkout_handle();
typedef RefCountHandle<Node> Handle;
Handle get();

private:
NodePtr acquire();
void await(Type version);
Type tip() const;
Type inc_tip();
NodePtr synchronize_with(Type version, NodePtr last) const;
DEBUG_ONLY(void assert_state(const Node* node) const;)
struct PaddedTip {
DEFINE_PAD_MINUS_SIZE(0, DEFAULT_CACHE_LINE_SIZE, 0);
volatile Type _value;
DEFINE_PAD_MINUS_SIZE(1, DEFAULT_CACHE_LINE_SIZE, sizeof(volatile Type));
};
PaddedTip _tip;
NodePtr _head;
volatile int _spinlock;

NodePtr acquire();
void release(NodePtr node);
void await(Type version);
Type tip() const;
Type increment();
NodePtr synchronize_with(Type version, NodePtr last) const;
debug_only(bool is_registered(Type version) const;)
friend class Handle;
};

#endif // SHARE_JFR_UTILITIES_JFRVERSIONSYSTEM_HPP

0 comments on commit 5cfb36e

Please sign in to comment.