JDK-8234773 : Fix ThreadsSMRSupport::_bootstrap_list
  • Type: Bug
  • Component: hotspot
  • Sub-Component: runtime
  • Affected Version: 14
  • Priority: P4
  • Status: Resolved
  • Resolution: Fixed
  • Submitted: 2019-11-25
  • Updated: 2021-01-14
  • Resolved: 2021-01-08
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 17
17 b05Fixed
Related Reports
Relates :  
Relates :  
Description
Currently, ThreadsSMRSupport::_bootstrap_list is a class static variable of type ThreadsList, with the following initializer.

ThreadsList ThreadsSMRSupport::_bootstrap_list = ThreadsList(0);

There are a couple of problems with this.

First, that is technically a copy initialization (C++03 8.5/14), with copy elision being applicable (C++03 12.8/15). The copy assignment operator must be accessible and defined, even if copy elision eliminates the reference. The copy ctor and assignment operator for ThreadsList are currently the defaults, so are not definitions one should be using (see below; ThreadsList should perhaps be noncopyable).  The only reason we don't see crashes here is copy elision, but that's an optional optimization; though widely implemented, elimination of the copy isn't required until C++17.

The copy initialization and associated reliance on copy elision can be eliminated by using direct initialization, e.g.

ThreadsList ThreadsSMRSupport::_bootstrap_list(0);

Note that declaring ThreadsList noncopyable via the technique of giving it declared but undefined copy ctor and assignment operator may not catch this specific case; ThreadsSMRSupport is a friend of ThreadsList, so has private access, and copy elision may then eliminate the reference.  Using C++11 deleted definitions would catch that.

However, there's a second issue here. A ThreadsList contains an array managed via NEW/FREE_C_HEAP_ARRAY. (This is why the default copy ctor and assignment operator shouldn't be used.) We normally avoid static initialization of allocation or (other) things which depend on VM initialization. It may be that the current implementation only works by accident of static initializer ordering.

Comments
Changeset: 10a6b0d9 Author: Kim Barrett <kbarrett@openjdk.org> Date: 2021-01-08 14:25:12 +0000 URL: https://git.openjdk.java.net/jdk/commit/10a6b0d9
08-01-2021

This prevents building with -fno-elide-constructors. While we don't normally build with that option (and wouldn't want to), it's a useful tool for investigating certain kinds of issues (JDK-8258010, for example).
03-01-2021

Erik: > We normally avoid static initialization of allocation or (other) things which depend on VM initialization. The issue is when allocation may involve other VM facilities, like NMT, and/or has other dependencies on VM initialization that may not have happened yet.
03-12-2019

If memory serves right, this list was added to dodge some annoying NULL checks all over the place, and instead have a sentinel empty list to be used early on (this list stuff was used surprisingly early in bootstrapping). Do we really not support dynamic allocations from static initializers? I thought we did. It seems to me that rather than having all users of dynamic memory lazily initialize their statics, the allocator for calling malloc should initialize itself lazily instead (so that it can be used from static initializers). That would be along the principle of least surprises, and making the VM more robust.
03-12-2019

[~eosterlund] and [~rehn] - do either of you remember the details behind the the _bootstrap_list field?
02-12-2019

Some possible solutions to the allocation at static initialization: (1) Change _bootstrap_list to a ThreadsList* initialized to NULL, and lazily construct the instance in the associated accessor. This also affects access to _java_thread_list, which is currently initialized to &_bootstrap_list and would need similar lazy initialization. (2) Rather than lazy initialization in accessors, add an initialization function that gets called sufficiently early in create_vm to initialize _bootstrap_list and _java_thread_list. (3) Eliminate the allocation by ThreadsList and instead give that class a pseudo-flexible-array member, changing how instances are allocated and freed accordingly.
25-11-2019