8256299: Implement JEP 396: Strongly Encapsulate JDK Internals by Default #1324

mbreinhold · 2020-11-19T16:49:30Z

Please review this implementation of JEP 396 (https://openjdk.java.net/jeps/396).
Alan Bateman is the original author; I’ve credited him in the commit metadata.
Passes tiers 1-3 on {linux,macos,windows}-x64 and linux-aarch64.

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed

Issue

JDK-8256299: Implement JEP 396: Strongly Encapsulate JDK Internals by Default

Reviewers

Mandy Chung (@mlchung - Reviewer) ⚠️ Review applies to baf34a1
Alan Bateman (@AlanBateman - Reviewer)

Contributors

Alan Bateman <alanb@openjdk.org>

Download

$ git fetch https://git.openjdk.java.net/jdk pull/1324/head:pull/1324
$ git checkout pull/1324

…ault

bridgekeeper · 2020-11-19T16:49:56Z

👋 Welcome back mr! 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 · 2020-11-19T16:50:53Z

@mbreinhold The following labels will be automatically applied to this pull request:

core-libs
hotspot-runtime

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

mbreinhold · 2020-11-19T16:51:20Z

/label jigsaw, core-libs

openjdk · 2020-11-19T16:51:39Z

@mbreinhold The label jigsaw is not a valid label. These labels are valid:

serviceability
hotspot
sound
hotspot-compiler
kulla
i18n
shenandoah
jdk
javadoc
2d
security
swing
hotspot-runtime
jmx
build
nio
beans
core-libs
compiler
net
hotspot-gc
hotspot-jfr
awt

mbreinhold · 2020-11-19T16:52:03Z

/csr

openjdk · 2020-11-19T16:52:25Z

@mbreinhold the issue for this pull request, JDK-8256299, already has an approved CSR request: JDK-8256300

mlbridge · 2020-11-19T16:55:11Z

Webrevs

test/lib-test/jdk/test/lib/hexdump/ASN1FormatterTest.java

test/jdk/tools/launcher/modules/illegalaccess/IllegalAccessTest.java

openjdk · 2020-11-19T19:53:31Z

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

8256299: Implement JEP 396: Strongly Encapsulate JDK Internals by Default

Co-authored-by: Alan Bateman <alanb@openjdk.org>
Reviewed-by: mchung, alanb

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

c47ab5f: 8256515: javax.xml.XMLEventReader produces incorrect START_DOCUMENT event
291ba97: 8251267: CDS tests should use CDSTestUtils.getOutputDir instead of System.getProperty("user.dir")
f48d5d1: 8257789: Fix incremental build of test-image and bundles
1a9ed92: 8200102: NativeLibraryTest.java fails intermittently, unloaded count is not same as expected
264feb3: 8257905: Make fixpath.sh more liberal in accepting paths embedded in arguments
044616b: 8252049: Native memory leak in ciMethodData ctor
fab6158: 8236413: AbstractConnectTimeout should tolerate both NoRouteToHostException and UnresolvedAddressException
936a7ac: 8252797: Non-PCH build fails on Ubuntu 16.4 when building with gtests
d0c5265: 8256149: Weird AST structure for incomplete member select
a708024: 8257194: Add 'foreign linker API' in 'jdk.incubator.foreign' module desc/summary
... and 331 more: https://git.openjdk.java.net/jdk/compare/675d1d56e67d2b875da0b3f6c614bb0851bc44db...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.

…dPackage test

dholmes-ora

The hostpot change is trivially fine.

I'm somewhat surprised by the extent of the other changes given the basic change here is to deprecate the option and change its default. I literally expected to see only 2 simple changes to the functional code: the deprecation warning and the changing of the default value.

AlanBateman · 2020-11-20T08:01:14Z

I'm somewhat surprised by the extent of the other changes given the basic change here is to deprecate the option and change its default. I literally expected to see only 2 simple changes to the functional code: the deprecation warning and the changing of the default value.

There are interactions with the archiving of the module graph and boot layer. Here's one of the mails from the review of JDK-8244778 that has more context:
https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-August/068159.html
The summary is that, as a short term measure until JEP 396 showed up, we agreed to have the maps to support exporting and opening packages for illegal reflective acces archived with the boot layer. Switching the default allows it goes away.

mlbridge · 2020-11-20T09:31:08Z

Mailing list message from David Holmes on hotspot-runtime-dev:

Hi Alan,

On 20/11/2020 6:04 pm, Alan Bateman wrote:

On Fri, 20 Nov 2020 07:33:50 GMT, David Holmes <dholmes at openjdk.org> wrote:

I'm somewhat surprised by the extent of the other changes given the basic change here is to deprecate the option and change its default. I literally expected to see only 2 simple changes to the functional code: the deprecation warning and the changing of the default value.

There are interactions with the archiving of the module graph and boot layer. Here's one of the mails from the review of JDK-8244778 that has more context:
https://mail.openjdk.java.net/pipermail/core-libs-dev/2020-August/068159.html
The summary is that, as a short term measure until JEP 396 showed up, we agreed to have the maps to support exporting and opening packages for illegal reflective acces archived with the boot layer. Switching the default allows it goes away.

Thanks for the information. I don't understand enough to know why this
is the case.

Cheers,
David

…dPackage test

…nExportedPackage test This reverts commit 6a4d479.

…dPackage test (v2)

mbreinhold · 2020-12-08T22:09:38Z

/contributor add alanb

openjdk · 2020-12-08T22:09:53Z

@mbreinhold
Contributor Alan Bateman <alanb@openjdk.org> successfully added.

mbreinhold · 2020-12-08T22:11:59Z

/integrate

openjdk · 2020-12-08T22:12:46Z

@mbreinhold Since your change was applied there have been 341 commits pushed to the master branch:

c47ab5f: 8256515: javax.xml.XMLEventReader produces incorrect START_DOCUMENT event
291ba97: 8251267: CDS tests should use CDSTestUtils.getOutputDir instead of System.getProperty("user.dir")
f48d5d1: 8257789: Fix incremental build of test-image and bundles
1a9ed92: 8200102: NativeLibraryTest.java fails intermittently, unloaded count is not same as expected
264feb3: 8257905: Make fixpath.sh more liberal in accepting paths embedded in arguments
044616b: 8252049: Native memory leak in ciMethodData ctor
fab6158: 8236413: AbstractConnectTimeout should tolerate both NoRouteToHostException and UnresolvedAddressException
936a7ac: 8252797: Non-PCH build fails on Ubuntu 16.4 when building with gtests
d0c5265: 8256149: Weird AST structure for incomplete member select
a708024: 8257194: Add 'foreign linker API' in 'jdk.incubator.foreign' module desc/summary
... and 331 more: https://git.openjdk.java.net/jdk/compare/675d1d56e67d2b875da0b3f6c614bb0851bc44db...master

Your commit was automatically rebased without conflicts.

Pushed as commit ed4c4ee.

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

8256299: Implement JEP 396: Strongly Encapsulate JDK Internals by Def…

4c0e888

…ault

openjdk bot added the rfr Pull request is ready for review label Nov 19, 2020

openjdk bot added hotspot-runtime hotspot-runtime-dev@openjdk.org core-libs core-libs-dev@openjdk.org labels Nov 19, 2020

AlanBateman reviewed Nov 19, 2020

View reviewed changes

test/lib-test/jdk/test/lib/hexdump/ASN1FormatterTest.java Outdated Show resolved Hide resolved

Remove unnecessary @module :open from ASN1FormatterTest.java

baf34a1

mlchung approved these changes Nov 19, 2020

View reviewed changes

test/jdk/tools/launcher/modules/illegalaccess/IllegalAccessTest.java Show resolved Hide resolved

openjdk bot added the ready Pull request is ready to be integrated label Nov 19, 2020

Add "Add-Exports" case for the setAccessibleNonPublicMemberNonExporte…

a57e606

…dPackage test

dholmes-ora reviewed Nov 20, 2020

View reviewed changes

mbreinhold added 2 commits November 23, 2020 15:02

Fix "Add-Exports" case for the setAccessibleNonPublicMemberNonExporte…

6a4d479

…dPackage test

Revert: Fix "Add-Exports" case for the setAccessibleNonPublicMemberNo…

1a7ab4e

…nExportedPackage test This reverts commit 6a4d479.

AlanBateman approved these changes Nov 24, 2020

View reviewed changes

Fix "Add-Exports" case for the setAccessibleNonPublicMemberNonExporte…

0c9654a

…dPackage test (v2)

AlanBateman approved these changes Nov 26, 2020

View reviewed changes

openjdk bot closed this Dec 8, 2020

openjdk bot added the integrated Pull request has been integrated label Dec 8, 2020

openjdk bot removed ready Pull request is ready to be integrated rfr Pull request is ready for review labels Dec 8, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

8256299: Implement JEP 396: Strongly Encapsulate JDK Internals by Default #1324

8256299: Implement JEP 396: Strongly Encapsulate JDK Internals by Default #1324

mbreinhold commented Nov 19, 2020 •

edited by openjdk bot

bridgekeeper bot commented Nov 19, 2020

openjdk bot commented Nov 19, 2020

mbreinhold commented Nov 19, 2020

openjdk bot commented Nov 19, 2020

mbreinhold commented Nov 19, 2020

openjdk bot commented Nov 19, 2020

mlbridge bot commented Nov 19, 2020 •

edited

openjdk bot commented Nov 19, 2020 •

edited

dholmes-ora left a comment

AlanBateman commented Nov 20, 2020

mlbridge bot commented Nov 20, 2020

mbreinhold commented Dec 8, 2020

openjdk bot commented Dec 8, 2020

mbreinhold commented Dec 8, 2020

openjdk bot commented Dec 8, 2020

8256299: Implement JEP 396: Strongly Encapsulate JDK Internals by Default #1324

8256299: Implement JEP 396: Strongly Encapsulate JDK Internals by Default #1324

Conversation

mbreinhold commented Nov 19, 2020 • edited by openjdk bot

Progress

Issue

Reviewers

Contributors

Download

bridgekeeper bot commented Nov 19, 2020

openjdk bot commented Nov 19, 2020

mbreinhold commented Nov 19, 2020

openjdk bot commented Nov 19, 2020

mbreinhold commented Nov 19, 2020

openjdk bot commented Nov 19, 2020

mlbridge bot commented Nov 19, 2020 • edited

Webrevs

openjdk bot commented Nov 19, 2020 • edited

dholmes-ora left a comment

Choose a reason for hiding this comment

AlanBateman commented Nov 20, 2020

mlbridge bot commented Nov 20, 2020

mbreinhold commented Dec 8, 2020

openjdk bot commented Dec 8, 2020

mbreinhold commented Dec 8, 2020

openjdk bot commented Dec 8, 2020

mbreinhold commented Nov 19, 2020 •

edited by openjdk bot

mlbridge bot commented Nov 19, 2020 •

edited

openjdk bot commented Nov 19, 2020 •

edited