JDK-8234192 : undefined behavior: C++ volatile keyword
  • Type: Enhancement
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: 15
  • Priority: P3
  • Status: Open
  • Resolution: Unresolved
  • Submitted: 2019-11-14
  • Updated: 2024-07-10
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.
Other
tbdUnresolved
Related Reports
Relates :  
Relates :  
Relates :  
Relates :  
Sub Tasks
JDK-8256474 :  
Description
This bug is specifically about undefined behavior from use of the C++ volatile keyword in the HotSpot source base. (Similar bugs may well be on file for other C or C++ source bases, or over other classes of undefined code.) 

There is a traditional meaning to volatile in C and C++ which boils down to "optimize loads and stores less vigorously, and use more expensive hardware operations if necessary".  The expensive hardware operations have sometimes conferred extra benefits, such as atomicity, in an ad hoc manner.  The good news is that this traditional meaning is being replaced by a proper memory model in recent years; the bad news for HotSpot is that this memory model is focused clearly on ordering of side effects and handles atomicity via other means.

Reference:  https://en.cppreference.com/w/cpp/language/cv

The Java Memory Model, which is similar to but predates the modern C++ memory model, differs in many details, including its treatment of the volatile keyword.  In the JMM volatile integrates/conflates atomicity and sequencing.  There is a special risk to volatile in HotSpot that a reader of C++ code may mistakenly assign a Java meaning to an occurrence of volatile in C++ code.

Another special risk to HotSpot from volatile is that, as the C++ community evolves the meaning of volatile, we will be tasked to support various platforms and toolchains that are at varying levels of adoption.  We've seen tools which are bleeding-edge, conservative, and stuck-in-the-past.  It's a challenge to pick a combination of platform settings and code styles to keep our source base up to date yet broadly portable.

The result is that, as the meaning of volatile is sharpened (and/or deprecated) in C++ and as it diverges more clearly from both tradition and the JMM, volatile risks losing behaviors that we rely on in our code base.  We need to re-evaluate our uses of the keyword and in some (or all?) uses replace them with recommended alternatives, such as explicit C++ atomics.

Broadly speaking, HotSpot tends to build small abstractions and utility functions to encapsulate portability risks.  It may be that we need to organize our code base so that all remaining uses of volatile (and atomics) are encapsulated by us in clearly documented, carefully tested, highly portable abstractions, and "naked volatile" is discouraged in all other parts of our code base.

# more data

## C++ memory model

In some versions of C++, use of a volatile variable to perform lock-free inter-thread synchronization is defined to be a data race.  Data races are undefined behavior, which is not something the JVM easily tolerates.

https://en.cppreference.com/w/cpp/language/memory_model
— See passage beginning "When an evaluation of an expression writes to a memory location and another evaluation reads or modifies the same memory location, the expressions are said to conflict." and ending "If a data race occurs, the behavior of the program is undefined."

https://en.cppreference.com/w/cpp/atomic/memory_order
— See passage including "...this order is not guaranteed to be observed by another thread, since volatile access does not establish inter-thread synchronization..."

## is volatile on the way out?

Some uses (stupid ones) are being deprecated.  What trend is this part of?
https://m.youtube.com/watch?v=KJW_DLaVXIY
CppCon 2019: JF Bastien “Deprecating volatile”

# in defense of volatile

The case for volatile is something like the following:

1. It’s a natural and familiar notation; folks know what it means.  (Counter: Maybe they don't anymore.)

2. if you don’t write an explicit “load” or “store” call (just x=*vp or some such) the compiler fixes it for you (instead of silently making a race condition).  (Counter:  It's too magic.  Hard to tell what "x=*vp" means from inspection if it's type-driven.)

3. Explicit code in this case is too explicit and verbose, hurtiing readability.  (Counter: Encapsulate it.)

Perhaps it boils down to this:  How do we want to formulate those parts of our code which are subject to races?  Can we isolate them inside appropriate abstractions?  Or will our workarounds be so ugly, clunky, noisy, and verbose that they make the problem of races worse than the volatile keyword, since they obscure the meaning of our code?

Comments
We have been talking about an eventual move to C++ atomics and memory model for a number of years now, but it never seems to get to a point where we are ready to action it. In part because the C++ memory model in particular has been so problematic (ref "release" doesn't really mean "release" [1]) and because the C++ standards committee keep changing their minds each time they update things (ref "consume" semantics added in C++11 IIRC, removed in C++17 but apparently back on the table for C++20!) As an interim step there are moves afoot to transition away from using volatile variables and plain loads/stores that we expect/require to be atomic (not torn) accesses (for aligned 32-bit and 64-bit variables at least), and to use our own Atomic::load and Atomic::store (which themselves rely on the same undefined semantics of volatile but at least it is more localised) - as Erik O. wrote to me earlier this year: "I try to use Atomic::load/store as good practice when I need atomicity, to reduce headache later on when we reroute Atomic::load/store to std::atomic." There was a related discussion in a recent review thread because Robbin E. was also recommending moving to this style. I have no concerns with adopting this style in the hotspot code, I just want to be sure that everyone is aware of it and understand the coding patterns being advised - to which end we should update the hotspot style guide wiki with a few before/after code examples. [1] http://preshing.com/20131125/acquire-and-release-fences-dont-work-the-way-youd-expect/
19-11-2019