8229867: Re-examine synchronization usages in http and https protocol handlers

[PoojaDP-23]

This is a backport request and this fix is a dependency for 8293562.

This is a fix that upgrades the old HTTP and HTTPS legacy stack to use virtual-thread friendly locking instead of synchronized monitors.

We have removed @java.io.serial in AuthenticationInfo.java since the serial.class file not located.

---

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-8229867: Re-examine synchronization usages in http and https protocol handlers

Reviewing

Using git

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

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

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 1710

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

Using diff file

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

Webrev

Link to Webrev Comment

[bridgekeeper]

Hi @PoojaDP-23, welcome to this OpenJDK project and thanks for contributing!

We do not recognize you as Contributor and need to ensure you have signed the Oracle Contributor Agreement (OCA). If you have not signed the OCA, please follow the instructions. Please fill in your GitHub username in the "Username" field of the application. Once you have signed the OCA, please let us know by writing /signed in a comment in this pull request.

If you already are an OpenJDK Author, Committer or Reviewer, please click here to open a new issue so that we can record that fact. Please use "Add GitHub user PoojaDP-23" as summary for the issue.

If you are contributing this work on behalf of your employer and your employer has signed the OCA, please let us know by writing /covered in a comment in this pull request.

[openjdk]

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

[PoojaDP-23]

/covered

[bridgekeeper]

Thank you! Please allow for a few business days to verify that your employer has signed the OCA. Also, please note that pull requests that are pending an OCA check will not usually be evaluated, so your patience is appreciated!

[robilad]

Hi, please send an e-mail to Dalibor.Topic@oracle.com so that I can mark your account as verified in Skara.

[PoojaDP-23]

@robilad - I have sent an email to Dalibor.Topic@oracle.com. Thanks

[mlbridge]

Webrevs

• 01: Full - Incremental (8084ffc4)
• 00: Full (822d883f)

[PoojaDP-23]

@robilad - Earlier Workflows aren't being run on this forked repository because it was not enabled. Now I have enabled the workflow but still tests are not running on my fork. Could you please let me know what action needs to be done by myside?
Thanks

[RealCLanger]

@robilad - Earlier Workflows aren't being run on this forked repository because it was not enabled. Now I have enabled the workflow but still tests are not running on my fork. Could you please let me know what action needs to be done by myside? Thanks

Hi @PoojaDP-23,
I guess the easiest way to get current GHA checks is to merge your branch with current master of jdk11u-dev and push it. This will trigger a run with the most current code.

[PoojaDP-23]

@RealCLanger - Could you please review the backport PR?

[RealCLanger]

@RealCLanger - Could you please review the backport PR?

I can have a look. But please do a merge with master before and by that, trigger GHA. Thanks.

[PoojaDP-23]

@RealCLanger - I have triggered GHA for my branch and master. I have merged my branch to master by following below steps:

git checkout master
git pull https://github.com/openjdk/jdk11u-dev master
git checkout <Your_BRANCH_NAME>
git merge master
git push origin HEAD

Could you please review the PR.

[PoojaDP-23]

@RealCLanger - Could you please review the PR? Thanks

[RealCLanger]

Hi @PoojaDP-23,

when looking at the scope of this backport and its successor, JDK-8293562, which I've understood is what you actually want to fix, I'm a bit concerned whether this is appropriate as a backport to JDK11. After all, it completely reworks the locking in the HTTP client and we can't be sure what side effects this might cause in other productive usages. It might even unveil issues that have not been discovered in head.

So, could you go back and give some more explanation what kind of bug you want to fix in 11u? Maybe we can find a less intrusive fix that could also differ from OpenJDK upstream? Maybe JDK-8293562 can be adapted for 11u without this one?

Thanks
Christoph

[PoojaDP-23]

Thanks for the update @RealCLanger

Yes, we need to backport JDK-8293562 to jdk-11 and backporting the dependency as well. Please let us know about the less intrusive fix, we shall evaluate if this resolves the issue.

Also, could you please help us understand the following ?

1. How this backport will be different from JDK17 as this is already backported to JDK17 and what/why are we expecting side effects in JDK11 ?

2. Unit test suite execution will be sufficient to confirm the adaptability of these two backports or what exact side effects to be concerned with.

Please share your response ASAP. It will help us to decide on next steps. thx

[GoeLin]

Dear @PoojaDP-23

Christoph already reasoned why he thinks this is not appropriate for 11: " After all, it completely reworks the locking in the HTTP client and we can't be sure what side effects this might cause in other productive usages. It might even unveil issues that have not been discovered in head."

You did not give a reason why you need these changes yet. Please reason for this carefully.

You ask "Please let us know about the less intrusive fix," It is up to you, the Contributor, to draft a less intrusive fix. If you can not do that yourself, you also can not judge the risk of the backport properly because you have not understood the fix.

ASAP is not a vocabulary used typically in open source projects where people work on a voluntary basis.

Please see https://wiki.openjdk.org/display/JDKUpdates/JDK11u for a description of eligible backports.

[GoeLin]

Also, you have failing tests.

[theRealAph]

No. This patch will never be accepted in JDK 11u.

[openjdk]

@PoojaDP-23 this pull request can not be integrated into master due to one or more merge conflicts. To resolve these merge conflicts and update this pull request you can run the following commands in the local repository for your personal fork:

git checkout jdk11-keepalive
git fetch https://git.openjdk.org/jdk11u-dev.git master
git merge FETCH_HEAD
# resolve conflicts and follow the instructions given by git merge
git commit -m "Merge master"
git push