8339540: Unify include requirements for PlatformMonitor/Mutex constructors/destructors

[stefank]

Today, if you write code that, potentially implicitly, calls the PlatformMonitor destructor you need to include os.inline.hpp to get it to compile on Windows but you don't need to do that for "posix" platforms.

That's because the destructor is located in os_posix.cpp for "posix" and in os_windows.inline.hpp for Windows.

This leads to the situation that you can get everything to compile on Linux, just for it to fail linking when you later build it on Windows. I propose that we simplify this and move the constructors and destructors to the cpp files.

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8339540: Unify include requirements for PlatformMonitor/Mutex constructors/destructors (Enhancement - P4)

Reviewers

• Coleen Phillimore (@coleenp - Reviewer)
• Stefan Johansson (@kstefanj - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/20851/head:pull/20851
$ git checkout pull/20851

Update a local copy of the PR:
$ git checkout pull/20851
$ git pull https://git.openjdk.org/jdk.git pull/20851/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 20851

View PR using the GUI difftool:
$ git pr show -t 20851

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/20851.diff

Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back stefank! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@stefank This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8339540: Unify include requirements for PlatformMonitor/Mutex constructors/destructors

Reviewed-by: coleenp, sjohanss

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 14 new commits pushed to the master branch:

• 28de44d: 8332461: ubsan : dependencies.cpp:906:3: runtime error: load of value 4294967295, which is not a valid value for type 'DepType'
• 96df5a6: 8339316: Test runtime/exceptionMsgs/NoClassDefFoundError/NoClassDefFoundErrorTest.java fails after JDK-8338257
• 55312e1: 8338937: Optimize the string concatenation of ClassDesc
• d4dfa01: 8338530: CDS warning Skipping java/lang/invoke/BoundMethodHandle$Species_LLLL
• b8d560b: 8339233: Test javax/swing/JButton/SwingButtonResizeTestWithOpenGL.java#id failed: Button renderings are different after window resize
• 1353601: 8338924: C1: assert(0 <= i && i < _len) failed: illegal index 5 for length 5
• 433f6d8: 8339492: StackMapDecoder::writeFrames makes lots of allocations
• ef96a7b: 8332901: Select{Current,New}ItemTest.java for Choice don't open popup on macOS
• 12d060a: 8339148: Make os::Linux::active_processor_count() public
• 0cfd08f: 8339112: Move JVM Klass flags out of AccessFlags
• ... and 4 more: https://git.openjdk.org/jdk/compare/4e2dde2f0d6f96d5f07020d2417189f411c4596a...master

As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[openjdk]

@stefank The following label will be automatically applied to this pull request:

• hotspot-runtime

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 00: Full (8039a62e)

[coleenp]

This seems trivial.

[kstefanj]

Looks good to me.

I ran into this problem working on an enhancement and I noticed that the failures only occur with --disable-precompiled-headers, which makes it even easier to accidentally push problematic code.

[stefank]

Thanks for the reviews!
/integrate

[openjdk]

Going to push as commit b389bb4.
Since your change was applied there have been 21 commits pushed to the master branch:

• ab656c3: 8339579: ZGC: Race results in only one of two remembered sets being cleared
• a505a1d: 8337951: Test sun/security/validator/samedn.sh CertificateNotYetValidException: NotBefore validation
• 6be9272: 8338591: Improve performance of MemorySegment::copy
• cb9f5c5: 8339290: Optimize ClassFile Utf8EntryImpl#writeTo
• 340e131: 8338971: IGV: Add incrementally inlined method name to phase name
• 2305d18: 8339384: Unintentional IOException in jdk.jdi module when JDWP end of stream occurs
• 96a0502: 8339369: G1: TestVerificationInConcurrentCycle.java fails with "Missing rem set entry" when using "-XX:G1RSetUpdatingPauseTimePercent=0 -XX:G1UpdateBufferSize=2"
• 28de44d: 8332461: ubsan : dependencies.cpp:906:3: runtime error: load of value 4294967295, which is not a valid value for type 'DepType'
• 96df5a6: 8339316: Test runtime/exceptionMsgs/NoClassDefFoundError/NoClassDefFoundErrorTest.java fails after JDK-8338257
• 55312e1: 8338937: Optimize the string concatenation of ClassDesc
• ... and 11 more: https://git.openjdk.org/jdk/compare/4e2dde2f0d6f96d5f07020d2417189f411c4596a...master

Your commit was automatically rebased without conflicts.

[openjdk]

@stefank Pushed as commit b389bb4.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.