8303942: os::write should write completely

[elifaslan1]

This is not a clean backport. There were multiple merge conflicts related to undefined parameters and existing methods. This backport is required to address a crash observed in Amazon production. Please refer to the related PR, which has already been merged.

In addition to GHA Tier 1, runtime hotspot, JFR, and hotspot GC tests have been run on macOS.

   TEST                                              TOTAL  PASS  FAIL ERROR
   jtreg:test/hotspot/jtreg:hotspot_runtime            880   880     0     0
==============================
TEST SUCCESS

Finished building target 'test' in configuration 'macosx-x86_64-server-release'

Additinonally, added clean backport of [JDK-8316468] (openjdk/jdk@7ce5bd1)

After new commit reran runtime hotspot tests and they all passed

---

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
• JDK-8303942 needs maintainer approval
• JDK-8183227 needs maintainer approval
• JDK-8316468 needs maintainer approval

Issues

• JDK-8303942: os::write should write completely (Bug - P4 - Approved)
• JDK-8316468: os::write incorrectly handles partial write (Bug - P2 - Approved)
• JDK-8183227: read/write APIs in class os shall return ssize_t (Enhancement - P4 - Approved)

Reviewers

• Paul Hohensee (@phohensee - Reviewer) ⚠️ Review applies to ce92b16d
• Aleksey Shipilev (@shipilev - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk17u-dev.git pull/2490/head:pull/2490
$ git checkout pull/2490

Update a local copy of the PR:
$ git checkout pull/2490
$ git pull https://git.openjdk.org/jdk17u-dev.git pull/2490/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 2490

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk17u-dev/pull/2490.diff

Webrev

Link to Webrev Comment

[bridgekeeper]

👋 Welcome back elifaslan1! 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]

@elifaslan1 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:

8303942: os::write should write completely
8316468: os::write incorrectly handles partial write
8183227: read/write APIs in class os shall return ssize_t

Reviewed-by: phh, shade

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 39 new commits pushed to the master branch:

• 17d65fe: 8305072: Win32ShellFolder2.compareTo is inconsistent
• ffa8a10: 8292948: JEditorPane ignores font-size styles in external linked css-file
• 541a284: 8322726: C2: Unloaded signature class kills argument value
• 43a6746: 8307091: A few client tests intermittently throw ConcurrentModificationException
• ce19e61: 8333724: Problem list security/infra/java/security/cert/CertPathValidator/certification/CAInterop.java#teliasonerarootcav1
• af699ff: 8325567: jspawnhelper without args fails with segfault
• a86a221: 8329995: Restricted access to /proc can cause JFR initialization to crash
• 28aa19e: 8316328: Test jdk/jfr/event/oldobject/TestSanityDefault.java times out for some heap sizes
• 9aa7b2f: 8279337: The MToolkit is still referenced in a few places
• 2db1501: 8323801: tag doesn't strikethrough the text
• ... and 29 more: https://git.openjdk.org/jdk17u-dev/compare/5cdff9f8c17868c5b66338fc1fa3df19eee266c5...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.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@phohensee, @shipilev) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

[openjdk]

This backport pull request has now been updated with issue from the original commit.

[mlbridge]

Webrevs

• 03: Full - Incremental (fe31415b)
• 02: Full - Incremental (828c0171)
• 01: Full - Incremental (fd5be8a9)
• 00: Full (808e194c)

[openjdk]

⚠️ @elifaslan1 This change is now ready for you to apply for maintainer approval. This can be done directly in each associated issue or by using the /approval command.

[elifaslan1]

/approval JDK-8303942 request Not a clean backport.There were multiple merge conflicts related to undefined parameters and existing methods. This backport is required to address a crash observed in Amazon production. Testing: hotspot,jfr and GHA tests passed.

[openjdk]

@elifaslan1
JDK-8303942: The approval request has been updated successfully.

[shipilev]

There is a P2 regression from this fix: JDK-8316468. It needs to go in this PR as well to avoid non-atomic breakage of 17u-dev.

[RealCLanger]

There is a P2 regression from this fix: JDK-8316468. It needs to go in this PR as well to avoid non-atomic breakage of 17u-dev.

Yes, please add this fix in here and request approval.

[elifaslan1]

/approval JDK-8303942 request Not a clean backport.There were multiple merge conflicts related to undefined parameters and existing methods. This backport is required to address a crash observed in Amazon production. Additionally JDK-8316468 regression has been adressed and added as a clean backport to the PR. Testing: hotspot,jfr and GHA tests passed.

[openjdk]

@elifaslan1
JDK-8303942: The approval request has been updated successfully.

[shipilev]

You need to say /issue add JDK-8316468 to record that this PR includes both fixes. Then also request the approval for that one with /approval 8316468 request ...

[elifaslan1]

/issue add JDK-8316468

[openjdk]

@elifaslan1
Adding additional issue to issue list: 8316468: os::write incorrectly handles partial write.

[elifaslan1]

/approval JDK-8316468 request clean backport to address a regression in os::write() that could cause heap dump corruption. Testing: hotspot,jfr and GHA tests passed.

[openjdk]

@elifaslan1
JDK-8316468: The approval request has been created successfully.

[shipilev]

This is a tad intrusive change, please make sure that all tests are pass. For example:

JTREG_KEYWORDS="\!headful & \!external-dep & \!printer" make test TEST=all

[elifaslan1]

I ran tests for both my branch and master , results look the same.

[elifaslan1]

/integrate

[shipilev]

Hold on a sec. Does this also include openjdk/jdk@4ff6720 ?

[openjdk]

⚠️ @elifaslan1 This pull request contains merges that bring in commits not present in the target repository. Since this is not a "merge style" pull request, these changes will be squashed when this pull request in integrated. If this is your intention, then please ignore this message. If you want to preserve the commit structure, you must change the title of this pull request to Merge <project>:<branch> where <project> is the name of another project in the OpenJDK organization (for example Merge jdk:master).

[openjdk]

@elifaslan1 Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. See OpenJDK Developers’ Guide for more information.

[elifaslan1]

Thanks Aleksey, it is much cleaner with this commit and there was only 3 conflicted files. First I applied the commit that you referred , after that I applied the last 2 commits.

[elifaslan1]

/issue add JDK-8183227

[openjdk]

@elifaslan1
Adding additional issue to issue list: 8183227: read/write APIs in class os shall return ssize_t.

[elifaslan1]

/approval JDK-8183227 request not clean backport. Current backport is dependent on this backport to address a regression in os::write() that could cause heap dump corruption. Testing: hotspot,jfr and GHA tests passed.

[openjdk]

@elifaslan1
JDK-8183227: The approval request has been created successfully.

[shipilev]

Thanks, I'll take a look at these three tomorrow.

[shipilev]

Looks good to me. Make sure to re-run all tests before integrating.

[elifaslan1]

/integrate

[openjdk]

@elifaslan1
Your change (at version fe31415) is now ready to be sponsored by a Committer.

[phohensee]

/sponsor

[openjdk]

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

• 17d65fe: 8305072: Win32ShellFolder2.compareTo is inconsistent
• ffa8a10: 8292948: JEditorPane ignores font-size styles in external linked css-file
• 541a284: 8322726: C2: Unloaded signature class kills argument value
• 43a6746: 8307091: A few client tests intermittently throw ConcurrentModificationException
• ce19e61: 8333724: Problem list security/infra/java/security/cert/CertPathValidator/certification/CAInterop.java#teliasonerarootcav1
• af699ff: 8325567: jspawnhelper without args fails with segfault
• a86a221: 8329995: Restricted access to /proc can cause JFR initialization to crash
• 28aa19e: 8316328: Test jdk/jfr/event/oldobject/TestSanityDefault.java times out for some heap sizes
• 9aa7b2f: 8279337: The MToolkit is still referenced in a few places
• 2db1501: 8323801: tag doesn't strikethrough the text
• ... and 29 more: https://git.openjdk.org/jdk17u-dev/compare/5cdff9f8c17868c5b66338fc1fa3df19eee266c5...master

Your commit was automatically rebased without conflicts.

[openjdk]

@phohensee @elifaslan1 Pushed as commit 2d9b1a6.

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