Skip to content

Commit

Permalink
8271931: Make AbortVMOnVMOperationTimeout more resilient to OS schedu…
Browse files Browse the repository at this point in the history
…ling

Reviewed-by: shade, dholmes
  • Loading branch information
albertnetymk committed Aug 9, 2021
1 parent a86ac0d commit 2f7a469
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 9 deletions.
27 changes: 21 additions & 6 deletions src/hotspot/share/runtime/vmThread.cpp
Expand Up @@ -60,8 +60,8 @@ void VMOperationTimeoutTask::task() {
if (is_armed()) {
jlong delay = nanos_to_millis(os::javaTimeNanos() - _arm_time);
if (delay > AbortVMOnVMOperationTimeoutDelay) {
fatal("VM operation took too long: " JLONG_FORMAT " ms elapsed since VM-op start (timeout: " INTX_FORMAT " ms)",
delay, AbortVMOnVMOperationTimeoutDelay);
fatal("%s VM operation took too long: " JLONG_FORMAT " ms elapsed since VM-op start (timeout: " INTX_FORMAT " ms)",
_vm_op_name, delay, AbortVMOnVMOperationTimeoutDelay);
}
}
}
Expand All @@ -70,13 +70,27 @@ bool VMOperationTimeoutTask::is_armed() {
return Atomic::load_acquire(&_armed) != 0;
}

void VMOperationTimeoutTask::arm() {
void VMOperationTimeoutTask::arm(const char* vm_op_name) {
_vm_op_name = vm_op_name;
_arm_time = os::javaTimeNanos();
Atomic::release_store_fence(&_armed, 1);
}

void VMOperationTimeoutTask::disarm() {
Atomic::release_store_fence(&_armed, 0);

// The two stores to `_armed` are counted in VM-op, but they should be
// insignificant compared to the actual VM-op duration.
jlong vm_op_duration = nanos_to_millis(os::javaTimeNanos() - _arm_time);

// Repeat the timeout-check logic on the VM thread, because
// VMOperationTimeoutTask might miss the arm-disarm window depending on
// the scheduling.
if (vm_op_duration > AbortVMOnVMOperationTimeoutDelay) {
fatal("%s VM operation took too long: completed in " JLONG_FORMAT " ms (timeout: " INTX_FORMAT " ms)",
_vm_op_name, vm_op_duration, AbortVMOnVMOperationTimeoutDelay);
}
_vm_op_name = nullptr;
}

//------------------------------------------------------------------------------------------------------------------
Expand Down Expand Up @@ -403,19 +417,20 @@ void VMThread::inner_execute(VM_Operation* op) {
_cur_vm_operation->name());

bool end_safepoint = false;
bool has_timeout_task = (_timeout_task != nullptr);
if (_cur_vm_operation->evaluate_at_safepoint() &&
!SafepointSynchronize::is_at_safepoint()) {
SafepointSynchronize::begin();
if (_timeout_task != NULL) {
_timeout_task->arm();
if (has_timeout_task) {
_timeout_task->arm(_cur_vm_operation->name());
}
end_safepoint = true;
}

evaluate_operation(_cur_vm_operation);

if (end_safepoint) {
if (_timeout_task != NULL) {
if (has_timeout_task) {
_timeout_task->disarm();
}
SafepointSynchronize::end();
Expand Down
6 changes: 3 additions & 3 deletions src/hotspot/share/runtime/vmThread.hpp
Expand Up @@ -39,15 +39,15 @@ class VMOperationTimeoutTask : public PeriodicTask {
private:
volatile int _armed;
jlong _arm_time;

const char* _vm_op_name;
public:
VMOperationTimeoutTask(size_t interval_time) :
PeriodicTask(interval_time), _armed(0), _arm_time(0) {}
PeriodicTask(interval_time), _armed(0), _arm_time(0), _vm_op_name(nullptr) {}

virtual void task();

bool is_armed();
void arm();
void arm(const char* vm_op_name);
void disarm();
};

Expand Down

1 comment on commit 2f7a469

@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.