8205399: Set node color on pinned HashMap.TreeNode deletion

[shipilev]

See the issue for details. It reproduces in 8u, which breaks applications that run with -ea -esa for extra safety. We have seen at least two in-production bug reports due to this.

The backport is not clean, because the test should reside in the different folder.

The verification barfs on discovering the red root, when it also discovers left/right child node is also red. This technically violates one of the basic properties of RB trees: a red node should not have red children. We can always change (sub-)root node color from red to black without violating RB properties, as the path to any descendant node would still have the same number of black nodes. AFAIU, leaving the (sub-)root red and the child red could potentially break something else. This is why I believe this backport is required and safe for 8u.

Additional testing:

• Linux x86_64, new regression test fails without the patch, passes with it
• Linux x86_64 jdk_util
• Linux x86_64 fastdebug tier1 (some intermittent failures, which look unrelated and present in current master)

---

Progress

• Change must not contain extraneous whitespace
• Commit message must refer to an issue

Issue

• JDK-8205399: Set node color on pinned HashMap.TreeNode deletion (Bug - P4)

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 325

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

Using diff file

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

Webrev

Link to Webrev Comment

[bridgekeeper]

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

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

[mlbridge]

Webrevs

• 01: Full - Incremental (cf1a17d6)
• 00: Full (78a88f3a)

[phohensee]

/clean

File location change only.

[openjdk]

@phohensee This backport pull request is now marked as clean

[openjdk]

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

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

8205399: Set node color on pinned HashMap.TreeNode deletion

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 no new commits pushed to the master branch. If another commit should be pushed before you perform the /integrate command, your PR will be automatically rebased. If you prefer to avoid any potential 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.

[shipilev]

I also do not see any relevant bugtail in HashMap.java up to JDK 21.

Hey @DougLea, are you good with this going in 8u?

[DougLea]

Yes, this was a rare bug that hadn't got checked for years in HashMap tests. There's no reason not to backport.

[shipilev]

Yes, this was a rare bug that hadn't got checked for years in HashMap tests. There's no reason not to backport.

Thank you!

[shipilev]

Testing is clean, push approval is here.

/integrate

[openjdk]

Going to push as commit a2a6873.

[openjdk]

@shipilev Pushed as commit a2a6873.

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