JDK-8246359 : clarify confusing comment in ObjectMonitor::EnterI()'s race with async deflation
  • Type: Enhancement
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: 15
  • Priority: P3
  • Status: Resolved
  • Resolution: Fixed
  • OS: generic
  • CPU: generic
  • Submitted: 2020-06-02
  • Updated: 2024-10-17
  • Resolved: 2020-06-02
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 15
15 b26Fixed
Related Reports
Relates :  
Description
The fix for JDK-8153224 includes a confusing comment about the
race that occurs between ObjectMonitor::EnterI() and the async
deflation code in deflate_monitor_using_JT().

Carsten raised this issue during the code review for JDK-8153224,
but we didn't reach resolution before JDK-8153224 was pushed.
Comments
URL: https://hg.openjdk.java.net/jdk/jdk/rev/2726fd3437fe User: dcubed Date: 2020-06-02 23:50:03 +0000
02-06-2020

Here's my first pass at a proposed rewrite for both occurrences of the comment: $ hg diff diff -r 629b14c63b75 src/hotspot/share/runtime/objectMonitor.cpp --- a/src/hotspot/share/runtime/objectMonitor.cpp Mon Jun 01 23:37:14 2020 -0400 +++ b/src/hotspot/share/runtime/objectMonitor.cpp Tue Jun 02 13:54:32 2020 -0400 @@ -526,12 +526,17 @@ if (AsyncDeflateIdleMonitors && try_set_owner_from(DEFLATER_MARKER, Self) == DEFLATER_MARKER) { - // Cancelled the in-progress async deflation. We bump contentions an - // extra time to prevent the async deflater thread from temporarily - // changing it to -max_jint and back to zero (no flicker to confuse - // is_being_async_deflated()). The async deflater thread will - // decrement contentions after it recognizes that the async - // deflation was cancelled. + // Cancelled the in-progress async deflation by changing owner from + // DEFLATER_MARKER to Self. As part of the contended enter protocol, + // contentions was incremented to a positive value before EnterI() + // was called and that prevents the deflater thread from winning the + // last part of the 2-part async deflation protocol. After EnterI() + // returns to enter(), contentions is decremented because the caller + // now owns the monitor. We bump contentions an extra time here to + // prevent the deflater thread from winning the last part of the + // 2-part async deflation protocol after the regular decrement + // occurs in enter(). The deflater thread will decrement contentions + // after it recognizes that the async deflation was cancelled. add_to_contentions(1); assert(_succ != Self, "invariant"); assert(_Responsible != Self, "invariant"); @@ -656,12 +661,17 @@ if (AsyncDeflateIdleMonitors && try_set_owner_from(DEFLATER_MARKER, Self) == DEFLATER_MARKER) { - // Cancelled the in-progress async deflation. We bump contentions an - // extra time to prevent the async deflater thread from temporarily - // changing it to -max_jint and back to zero (no flicker to confuse - // is_being_async_deflated()). The async deflater thread will - // decrement contentions after it recognizes that the async - // deflation was cancelled. + // Cancelled the in-progress async deflation by changing owner from + // DEFLATER_MARKER to Self. As part of the contended enter protocol, + // contentions was incremented to a positive value before EnterI() + // was called and that prevents the deflater thread from winning the + // last part of the 2-part async deflation protocol. After EnterI() + // returns to enter(), contentions is decremented because the caller + // now owns the monitor. We bump contentions an extra time here to + // prevent the deflater thread from winning the last part of the + // 2-part async deflation protocol after the regular decrement + // occurs in enter(). The deflater thread will decrement contentions + // after it recognizes that the async deflation was cancelled. add_to_contentions(1); break; }
02-06-2020