Skip to content

Commit

Permalink
8323438: Enhance assertions for Windows sync API failures
Browse files Browse the repository at this point in the history
Reviewed-by: ccheung, jwaters, dcubed
  • Loading branch information
David Holmes committed Jan 23, 2024
1 parent 52523d3 commit 5a74c2a
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 15 deletions.
12 changes: 9 additions & 3 deletions src/hotspot/os/windows/attachListener_windows.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2005, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2005, 2024, 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
Expand Down Expand Up @@ -248,10 +248,13 @@ Win32AttachOperation* Win32AttachListener::dequeue() {
DWORD res = ::WaitForSingleObject(enqueued_ops_semaphore(), INFINITE);
// returning from WaitForSingleObject will have decreased
// the current count of the semaphore by 1.
guarantee(res == WAIT_OBJECT_0, "wait failed");
guarantee(res != WAIT_FAILED, "WaitForSingleObject failed with error code: %lu", GetLastError());
guarantee(res == WAIT_OBJECT_0, "WaitForSingleObject failed with return value: %lu", res);

res = ::WaitForSingleObject(mutex(), INFINITE);
guarantee(res == WAIT_OBJECT_0, "wait failed");
guarantee(res != WAIT_FAILED, "WaitForSingleObject failed with error code: %lu", GetLastError());
guarantee(res == WAIT_OBJECT_0, "WaitForSingleObject failed with return value: %lu", res);


Win32AttachOperation* op = head();
if (op != nullptr) {
Expand Down Expand Up @@ -338,6 +341,9 @@ void Win32AttachOperation::complete(jint result, bufferedStream* result_stream)
}

DWORD res = ::WaitForSingleObject(Win32AttachListener::mutex(), INFINITE);
assert(res != WAIT_FAILED, "WaitForSingleObject failed with error code: %lu", GetLastError());
assert(res == WAIT_OBJECT_0, "WaitForSingleObject failed with return value: %lu", res);

if (res == WAIT_OBJECT_0) {

// put the operation back on the available list
Expand Down
33 changes: 23 additions & 10 deletions src/hotspot/os/windows/os_windows.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1997, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1997, 2024, 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
Expand Down Expand Up @@ -5354,7 +5354,8 @@ int PlatformEvent::park(jlong Millis) {
phri = new HighResolutionInterval(prd);
}
rv = ::WaitForSingleObject(_ParkHandle, prd);
assert(rv == WAIT_OBJECT_0 || rv == WAIT_TIMEOUT, "WaitForSingleObject failed");
assert(rv != WAIT_FAILED, "WaitForSingleObject failed with error code: %lu", GetLastError());
assert(rv == WAIT_OBJECT_0 || rv == WAIT_TIMEOUT, "WaitForSingleObject failed with return value: %lu", rv);
if (rv == WAIT_TIMEOUT) {
Millis -= prd;
}
Expand Down Expand Up @@ -5393,7 +5394,8 @@ void PlatformEvent::park() {
// spin attempts by this thread.
while (_Event < 0) {
DWORD rv = ::WaitForSingleObject(_ParkHandle, INFINITE);
assert(rv == WAIT_OBJECT_0, "WaitForSingleObject failed");
assert(rv != WAIT_FAILED, "WaitForSingleObject failed with error code: %lu", GetLastError());
assert(rv == WAIT_OBJECT_0, "WaitForSingleObject failed with return value: %lu", rv);
}

// Usually we'll find _Event == 0 at this point, but as
Expand Down Expand Up @@ -5456,16 +5458,25 @@ void Parker::park(bool isAbsolute, jlong time) {
JavaThread* thread = JavaThread::current();

// Don't wait if interrupted or already triggered
if (thread->is_interrupted(false) ||
WaitForSingleObject(_ParkHandle, 0) == WAIT_OBJECT_0) {
if (thread->is_interrupted(false)) {
ResetEvent(_ParkHandle);
return;
} else {
ThreadBlockInVM tbivm(thread);
OSThreadWaitState osts(thread->osthread(), false /* not Object.wait() */);
DWORD rv = WaitForSingleObject(_ParkHandle, 0);
assert(rv != WAIT_FAILED, "WaitForSingleObject failed with error code: %lu", GetLastError());
assert(rv == WAIT_OBJECT_0 || rv == WAIT_TIMEOUT, "WaitForSingleObject failed with return value: %lu", rv);
if (rv == WAIT_OBJECT_0) {
ResetEvent(_ParkHandle);
return;
} else {
ThreadBlockInVM tbivm(thread);
OSThreadWaitState osts(thread->osthread(), false /* not Object.wait() */);

WaitForSingleObject(_ParkHandle, time);
ResetEvent(_ParkHandle);
rv = WaitForSingleObject(_ParkHandle, time);
assert(rv != WAIT_FAILED, "WaitForSingleObject failed with error code: %lu", GetLastError());
assert(rv == WAIT_OBJECT_0 || rv == WAIT_TIMEOUT, "WaitForSingleObject failed with return value: %lu", rv);
ResetEvent(_ParkHandle);
}
}
}

Expand Down Expand Up @@ -5541,7 +5552,9 @@ int os::fork_and_exec(const char* cmd) {

if (rslt) {
// Wait until child process exits.
WaitForSingleObject(pi.hProcess, INFINITE);
DWORD rv = WaitForSingleObject(pi.hProcess, INFINITE);
assert(rv != WAIT_FAILED, "WaitForSingleObject failed with error code: %lu", GetLastError());
assert(rv == WAIT_OBJECT_0, "WaitForSingleObject failed with return value: %lu", rv);

GetExitCodeProcess(pi.hProcess, &exit_code);

Expand Down
5 changes: 3 additions & 2 deletions src/hotspot/os/windows/threadCritical_windows.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2001, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2001, 2024, 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
Expand Down Expand Up @@ -64,7 +64,8 @@ ThreadCritical::ThreadCritical() {
if (lock_owner != current_thread) {
// Grab the lock before doing anything.
DWORD ret = WaitForSingleObject(lock_event, INFINITE);
assert(ret == WAIT_OBJECT_0, "unexpected return value from WaitForSingleObject");
assert(ret != WAIT_FAILED, "WaitForSingleObject failed with error code: %lu", GetLastError());
assert(ret == WAIT_OBJECT_0, "WaitForSingleObject failed with return value: %lu", ret);
lock_owner = current_thread;
}
// Atomicity isn't required. Bump the recursion count.
Expand Down

0 comments on commit 5a74c2a

Please sign in to comment.