archive/jdk
1
0
Fork 0
mirror of https://github.com/openjdk/jdk.git synced 2025-08-25 22:04:51 +02:00
Code Issues Projects Releases Packages Wiki Activity Actions

8028073: race condition in ObjectMonitor implementation causing deadlocks

Browse source 
Move redo of ParkEvent.unpark() after JVMTI_EVENT_MONITOR_WAITED event handler is called.

Reviewed-by: dholmes, sspitsyn, dice, acorn
This commit is contained in:
Daniel D. Daugherty 2014-02-10 12:48:54 -08:00
parent dd25d6fed0
commit cf5c3370a3
2 changed files with 47 additions and 17 deletions
Download patch file Download diff file Expand all files Collapse all files

56
hotspot/src/share/vm/runtime/objectMonitor.cpp
View file

@ -1,5 +1,5 @@
/*
* Copyright (c) 1998, 2013, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 1998, 2014, 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
@ -382,6 +382,12 @@ void ATTR ObjectMonitor::enter(TRAPS) {
DTRACE_MONITOR_PROBE(contended__enter, this, object(), jt);
if (JvmtiExport::should_post_monitor_contended_enter()) {
JvmtiExport::post_monitor_contended_enter(jt, this);
// The current thread does not yet own the monitor and does not
// yet appear on any queues that would get it made the successor.
// This means that the JVMTI_EVENT_MONITOR_CONTENDED_ENTER event
// handler cannot accidentally consume an unpark() meant for the
// ParkEvent associated with this ObjectMonitor.
}
OSThreadContendState osts(Self->osthread());
@ -439,6 +445,12 @@ void ATTR ObjectMonitor::enter(TRAPS) {
DTRACE_MONITOR_PROBE(contended__entered, this, object(), jt);
if (JvmtiExport::should_post_monitor_contended_entered()) {
JvmtiExport::post_monitor_contended_entered(jt, this);
// The current thread already owns the monitor and is not going to
// call park() for the remainder of the monitor enter protocol. So
// it doesn't matter if the JVMTI_EVENT_MONITOR_CONTENDED_ENTERED
// event handler consumed an unpark() issued by the thread that
// just exited the monitor.
}
if (event.should_commit()) {
@ -1456,6 +1468,14 @@ void ObjectMonitor::wait(jlong millis, bool interruptible, TRAPS) {
// Note: 'false' parameter is passed here because the
// wait was not timed out due to thread interrupt.
JvmtiExport::post_monitor_waited(jt, this, false);
// In this short circuit of the monitor wait protocol, the
// current thread never drops ownership of the monitor and
// never gets added to the wait queue so the current thread
// cannot be made the successor. This means that the
// JVMTI_EVENT_MONITOR_WAITED event handler cannot accidentally
// consume an unpark() meant for the ParkEvent associated with
// this ObjectMonitor.
}
if (event.should_commit()) {
post_monitor_wait_event(&event, 0, millis, false);
@ -1499,21 +1519,6 @@ void ObjectMonitor::wait(jlong millis, bool interruptible, TRAPS) {
exit (true, Self) ; // exit the monitor
guarantee (_owner != Self, "invariant") ;
// As soon as the ObjectMonitor's ownership is dropped in the exit()
// call above, another thread can enter() the ObjectMonitor, do the
// notify(), and exit() the ObjectMonitor. If the other thread's
// exit() call chooses this thread as the successor and the unpark()
// call happens to occur while this thread is posting a
// MONITOR_CONTENDED_EXIT event, then we run the risk of the event
// handler using RawMonitors and consuming the unpark().
//
// To avoid the problem, we re-post the event. This does no harm
// even if the original unpark() was not consumed because we are the
// chosen successor for this monitor.
if (node._notified != 0 && _succ == Self) {
node._event->unpark();
}
// The thread is on the WaitSet list - now park() it.
// On MP systems it's conceivable that a brief spin before we park
// could be profitable.
@ -1595,6 +1600,25 @@ void ObjectMonitor::wait(jlong millis, bool interruptible, TRAPS) {
// post monitor waited event. Note that this is past-tense, we are done waiting.
if (JvmtiExport::should_post_monitor_waited()) {
JvmtiExport::post_monitor_waited(jt, this, ret == OS_TIMEOUT);
if (node._notified != 0 && _succ == Self) {
// In this part of the monitor wait-notify-reenter protocol it
// is possible (and normal) for another thread to do a fastpath
// monitor enter-exit while this thread is still trying to get
// to the reenter portion of the protocol.
//
// The ObjectMonitor was notified and the current thread is
// the successor which also means that an unpark() has already
// been done. The JVMTI_EVENT_MONITOR_WAITED event handler can
// consume the unpark() that was done when the successor was
// set because the same ParkEvent is shared between Java
// monitors and JVM/TI RawMonitors (for now).
//
// We redo the unpark() to ensure forward progress, i.e., we
// don't want all pending threads hanging (parked) with none
// entering the unlocked monitor.
node._event->unpark();
}
}
if (event.should_commit()) {