RFR: 8255595: delay_to_keep_mmu passes wrong arguments to Monitor wait
Please review this trivial fix of the second argument in a call to MonitorLocker::wait. Because the locker is constructed as no-safepoint-checking, the improper second argument is ignored rather than possibly going into an unexpected state. ------------- Commit messages: - fix call to wait Changes: https://git.openjdk.java.net/jdk/pull/934/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=934&range=00 Issue: https://bugs.openjdk.java.net/browse/JDK-8255595 Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod Patch: https://git.openjdk.java.net/jdk/pull/934.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/934/head:pull/934 PR: https://git.openjdk.java.net/jdk/pull/934
On Thu, 29 Oct 2020 13:56:20 GMT, Kim Barrett <kbarrett@openjdk.org> wrote:
Please review this trivial fix of the second argument in a call to MonitorLocker::wait. Because the locker is constructed as no-safepoint-checking, the improper second argument is ignored rather than possibly going into an unexpected state.
Looks good! ------------- Marked as reviewed by sjohanss (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/934
On Thu, 29 Oct 2020 13:56:20 GMT, Kim Barrett <kbarrett@openjdk.org> wrote:
Please review this trivial fix of the second argument in a call to MonitorLocker::wait. Because the locker is constructed as no-safepoint-checking, the improper second argument is ignored rather than possibly going into an unexpected state.
Lgtm and trivial. ------------- Marked as reviewed by tschatzl (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/934
On Thu, 29 Oct 2020 15:04:06 GMT, Thomas Schatzl <tschatzl@openjdk.org> wrote:
Please review this trivial fix of the second argument in a call to MonitorLocker::wait. Because the locker is constructed as no-safepoint-checking, the improper second argument is ignored rather than possibly going into an unexpected state.
Lgtm and trivial.
I wonder if it's feasible and/or worthwhile to use `clang-tidy` (or sth similar) to detect this kind of error in existing code and prevent it from happening again in new code. ------------- PR: https://git.openjdk.java.net/jdk/pull/934
On Thu, 29 Oct 2020 13:56:20 GMT, Kim Barrett <kbarrett@openjdk.org> wrote:
Please review this trivial fix of the second argument in a call to MonitorLocker::wait. Because the locker is constructed as no-safepoint-checking, the improper second argument is ignored rather than possibly going into an unexpected state.
Marked as reviewed by ayang (Author). ------------- PR: https://git.openjdk.java.net/jdk/pull/934
On Thu, 29 Oct 2020 15:42:05 GMT, Albert Mingkun Yang <ayang@openjdk.org> wrote:
Please review this trivial fix of the second argument in a call to MonitorLocker::wait. Because the locker is constructed as no-safepoint-checking, the improper second argument is ignored rather than possibly going into an unexpected state.
Marked as reviewed by ayang (Author).
I wonder if it's feasible and/or worthwhile to use `clang-tidy` (or sth similar) to detect this kind of error in existing code and prevent it from happening again in new code.
See https://bugs.openjdk.java.net/browse/JDK-8255596 ------------- PR: https://git.openjdk.java.net/jdk/pull/934
On Fri, 30 Oct 2020 04:09:53 GMT, Kim Barrett <kbarrett@openjdk.org> wrote:
Thank you, Kim. ------------- PR: https://git.openjdk.java.net/jdk/pull/934
Please review this trivial fix of the second argument in a call to MonitorLocker::wait. Because the locker is constructed as no-safepoint-checking, the improper second argument is ignored rather than possibly going into an unexpected state.
Kim Barrett has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains two additional commits since the last revision: - Merge branch 'master' into bad_wait - fix call to wait ------------- Changes: - all: https://git.openjdk.java.net/jdk/pull/934/files - new: https://git.openjdk.java.net/jdk/pull/934/files/b42f992c..216d75c4 Webrevs: - full: https://webrevs.openjdk.java.net/?repo=jdk&pr=934&range=01 - incr: https://webrevs.openjdk.java.net/?repo=jdk&pr=934&range=00-01 Stats: 875 lines in 57 files changed: 465 ins; 119 del; 291 mod Patch: https://git.openjdk.java.net/jdk/pull/934.diff Fetch: git fetch https://git.openjdk.java.net/jdk pull/934/head:pull/934 PR: https://git.openjdk.java.net/jdk/pull/934
On Thu, 29 Oct 2020 13:56:20 GMT, Kim Barrett <kbarrett@openjdk.org> wrote:
Please review this trivial fix of the second argument in a call to MonitorLocker::wait. Because the locker is constructed as no-safepoint-checking, the improper second argument is ignored rather than possibly going into an unexpected state.
This pull request has now been integrated. Changeset: 379ba80e Author: Kim Barrett <kbarrett@openjdk.org> URL: https://git.openjdk.java.net/jdk/commit/379ba80e Stats: 1 line in 1 file changed: 0 ins; 0 del; 1 mod 8255595: delay_to_keep_mmu passes wrong arguments to Monitor wait Remove improper wait argument. Reviewed-by: sjohanss, tschatzl, ayang ------------- PR: https://git.openjdk.java.net/jdk/pull/934
participants (4)
-
Albert Mingkun Yang
-
Kim Barrett
-
Stefan Johansson
-
Thomas Schatzl