Skip to content

Commit

Permalink
8255119: Monitor::wait takes signed integer as timeout
Browse files Browse the repository at this point in the history
Reviewed-by: jsjolen, stuefe, coleenp, gziemski
  • Loading branch information
David Holmes committed Jan 10, 2023
1 parent 4395578 commit a638663
Show file tree
Hide file tree
Showing 6 changed files with 24 additions and 23 deletions.
4 changes: 2 additions & 2 deletions src/hotspot/os/posix/mutex_posix.hpp
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1997, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1997, 2023, 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 @@ -133,7 +133,7 @@ class PlatformMonitor : public PlatformMutex {
NONCOPYABLE(PlatformMonitor);

public:
int wait(jlong millis);
int wait(uint64_t millis);
void notify();
void notify_all();
};
Expand Down
12 changes: 6 additions & 6 deletions src/hotspot/os/posix/os_posix.cpp
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1999, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1999, 2023, 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 @@ -1869,17 +1869,17 @@ PlatformMonitor::~PlatformMonitor() {
#endif // PLATFORM_MONITOR_IMPL_INDIRECT

// Must already be locked
int PlatformMonitor::wait(jlong millis) {
assert(millis >= 0, "negative timeout");
int PlatformMonitor::wait(uint64_t millis) {
if (millis > 0) {
struct timespec abst;
// We have to watch for overflow when converting millis to nanos,
// but if millis is that large then we will end up limiting to
// MAX_SECS anyway, so just do that here.
// MAX_SECS anyway, so just do that here. This also handles values
// larger than int64_t max.
if (millis / MILLIUNITS > MAX_SECS) {
millis = jlong(MAX_SECS) * MILLIUNITS;
millis = uint64_t(MAX_SECS) * MILLIUNITS;
}
to_abstime(&abst, millis_to_nanos(millis), false, false);
to_abstime(&abst, millis_to_nanos(int64_t(millis)), false, false);

int ret = OS_TIMEOUT;
int status = pthread_cond_timedwait(cond(), mutex(), &abst);
Expand Down
2 changes: 1 addition & 1 deletion src/hotspot/os/windows/mutex_windows.hpp
Expand Up @@ -56,7 +56,7 @@ class PlatformMonitor : public PlatformMutex {
public:
PlatformMonitor();
~PlatformMonitor();
int wait(jlong millis);
int wait(uint64_t millis);
void notify();
void notify_all();
};
Expand Down
11 changes: 7 additions & 4 deletions src/hotspot/os/windows/os_windows.cpp
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1997, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1997, 2023, 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 @@ -5358,11 +5358,14 @@ void Parker::unpark() {
// Platform Monitor implementation

// Must already be locked
int PlatformMonitor::wait(jlong millis) {
assert(millis >= 0, "negative timeout");
int PlatformMonitor::wait(uint64_t millis) {
int ret = OS_TIMEOUT;
// The timeout parameter for SleepConditionVariableCS is a DWORD
if (millis > UINT_MAX) {
millis = UINT_MAX;
}
int status = SleepConditionVariableCS(&_cond, &_mutex,
millis == 0 ? INFINITE : millis);
millis == 0 ? INFINITE : (DWORD)millis);
if (status != 0) {
ret = OS_OK;
}
Expand Down
12 changes: 5 additions & 7 deletions src/hotspot/share/runtime/mutex.cpp
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1998, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1998, 2023, 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 @@ -209,11 +209,10 @@ void Monitor::notify_all() {
_lock.notify_all();
}

bool Monitor::wait_without_safepoint_check(int64_t timeout) {
// timeout is in milliseconds - with zero meaning never timeout
bool Monitor::wait_without_safepoint_check(uint64_t timeout) {
Thread* const self = Thread::current();

// timeout is in milliseconds - with zero meaning never timeout
assert(timeout >= 0, "negative timeout");
assert_owner(self);
check_rank(self);

Expand All @@ -229,13 +228,12 @@ bool Monitor::wait_without_safepoint_check(int64_t timeout) {
return wait_status != 0; // return true IFF timeout
}

bool Monitor::wait(int64_t timeout) {
// timeout is in milliseconds - with zero meaning never timeout
bool Monitor::wait(uint64_t timeout) {
JavaThread* const self = JavaThread::current();
// Safepoint checking logically implies an active JavaThread.
assert(self->is_active_Java_thread(), "invariant");

// timeout is in milliseconds - with zero meaning never timeout
assert(timeout >= 0, "negative timeout");
assert_owner(self);
check_rank(self);

Expand Down
6 changes: 3 additions & 3 deletions src/hotspot/share/runtime/mutex.hpp
@@ -1,5 +1,5 @@
/*
* Copyright (c) 1998, 2022, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1998, 2023, 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 @@ -212,8 +212,8 @@ class Monitor : public Mutex {
// Wait until monitor is notified (or times out).
// Defaults are to make safepoint checks, wait time is forever (i.e.,
// zero). Returns true if wait times out; otherwise returns false.
bool wait(int64_t timeout = 0);
bool wait_without_safepoint_check(int64_t timeout = 0);
bool wait(uint64_t timeout = 0);
bool wait_without_safepoint_check(uint64_t timeout = 0);
void notify();
void notify_all();
};
Expand Down

1 comment on commit a638663

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