8273101: Eliminate the usage of threadgroup sandboxing in the java.util.logging

[mrserb]

At the time Java supported applets and webstart, a special mechanism for launching various applications in one JVM was used to reduce memory usage and each application was isolated from each other.

This isolation was implemented via ThreadGroups where each application created its own thread group and all data for the application was stored in the context of its own group.

Some parts of the JDK used ThreadGroups directly, others used special classes like sun.awt.AppContext.

Such sandboxing became useless since the plugin and webstart are no longer supported and were removed in jdk11.

Note that this is just a first step for the overall cleanup, here is my plan:

1. Cleanup the usage of AppContext in the “java.util.logging.LogManager" class in the java.base module.
2. Cleanup the usage of TheadGroup in the JavaSound
3. Cleanup the usage of TheadGroup in the JavaBeans
4. Cleanup the usage of AppContext in the Swing
5. Cleanup the usage of AppContext in the AWT
6. Delete the AppContext class.

The current PR is for the first step. So this is the request to delete the usage of AppContext in the LogManager and related tests.

Tested by tier1/tier2/tier3 and jdk_desktop tests.

---

Progress

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

Issue

• JDK-8273101: Eliminate the usage of threadgroup sandboxing in the java.util.logging

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.java.net/jdk pull/5326/head:pull/5326
$ git checkout pull/5326

Update a local copy of the PR:
$ git checkout pull/5326
$ git pull https://git.openjdk.java.net/jdk pull/5326/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 5326

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

Using diff file

Download this PR as a diff file:
https://git.openjdk.java.net/jdk/pull/5326.diff

[bridgekeeper]

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

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

• awt
• core-libs
• security

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.

[mlbridge]

Webrevs

• 01: Full (faf00ba0)
• 00: Full (8791d35b)

[prrace]

This fix requires coordinated closed changes so needs an Oracle sponsor.
And actually is probably better handled entirely by an Oracle engineer because pushes need to be very co-ordinated.

[prrace]

/reviewers 2

[openjdk]

@prrace
The number of required reviews for this PR is now set to 2 (with at least 1 of role reviewers).

[prrace]

I believe we should have a CSR for this. It removes a long standing capability in the platform that was used by
components such as plugin and webstart.

/csr

[openjdk]

@prrace has indicated that a compatibility and specification (CSR) request is needed for this pull request.
@mrserb please create a CSR request for issue JDK-8273101. This pull request cannot be integrated until the CSR request is approved.

[prrace]

Hmm I was under the impression this was removing AppContext itself but it is just removing the backdoor needed by logging
Perhaps this isn't the change that requires the CSR but it then leaves an inconsistent state where desktop supports AppContext still but other modules don't ...

[prrace]

@aivanov-jdk will help make sure the closed changes are pushed at exactly the same time as this gets pushed.

[prrace]

/csr unneeded

[openjdk]

@prrace determined that a CSR request is not needed for this pull request.

[mrserb]

Perhaps this isn't the change that requires the CSR but it then leaves an inconsistent state where desktop supports AppContext still but other modules don't ...

Even java.desktop does not support it fully, since for a couple of years nobody care about appcontext when write a new code.
Note that I mentioned the "threadgroup sandboxing" in the subject, which is not necessary implemented via Appcontext. The JavaBeans and JavaSound use the thread group directly, I plan to remove them as well.

[dfuch]

Right. I am also a bit uncomfortable about the inconsistency. That said - if everybody agrees that this should be done I have no objection. The changes to the java.util.logging implementation correspond to my expectation if this were to be carried through.

[dfuch]

Humm... There's no direct reference to AppContext here. Maybe this test should be preserved?

[mrserb]

It uses it indirectly, the next line under security manager trigger creation of the appcontext:
ImageIO.read(is); // triggers calls to system loggers & creation of main AppContext

but I can preserve/rename it for sure.

[AlanBateman]

No objection to removing this legacy/unused code but I think it would be useful to useful to have the JBS issue or the PR summary provide a bit more context. As I see it, this is just one piece of the overall cleanup and I assume there are more substantive changes to the java.desktop module coming, is that right?

[mrserb]

As I see it, this is just one piece of the overall cleanup and I assume there are more substantive changes to the java.desktop module coming, is that right?

Yes, you are right.

[mlchung]

Hmm I was under the impression this was removing AppContext itself but it is just removing the backdoor needed by logging
Perhaps this isn't the change that requires the CSR but it then leaves an inconsistent state where desktop supports AppContext still but other modules don't ...

What is the plan to remove AppContext completely from java.desktop? It's okay to have a separate PR to remove the logging dependency on AppContext but I'd prefer to see AppContext be removed about the same time with this PR.

[mrserb]

I'll remove its used one by one from the external usage like in "java.base" here, then in Swing, then in 2D like fonts, then last in AWT.

That change should not break something other than the tests which intentionally create different appcontexts. And it's easy to review.

[mrserb]

No objection to removing this legacy/unused code but I think it would be useful to useful to have the JBS issue or the PR summary provide a bit more context. As I see it, this is just one piece of the overall cleanup and I assume there are more substantive changes to the java.desktop module coming, is that right?

I have updated the PR and JBS

[mrserb]

Does anybody have any other suggestions?

[bridgekeeper]

@mrserb This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

[mrserb]

Does anybody have any other suggestions?

[mlchung]

The change looks okay. My suggestion is to get 1-6 all ready to push around the same time. It's okay to have separate JBS issues and PRs.

[mrserb]

The change looks okay. My suggestion is to get 1-6 all ready to push around the same time. It's okay to have separate JBS issues and PRs.

ok, I'll continue to work using the plan from the description.

[bridgekeeper]

@mrserb This pull request has been inactive for more than 4 weeks and will be automatically closed if another 4 weeks passes without any activity. To avoid this, simply add a new comment to the pull request. Feel free to ask for assistance if you need help with progressing this pull request towards integration!

[bridgekeeper]

@mrserb This pull request has been inactive for more than 8 weeks and will now be automatically closed. If you would like to continue working on this pull request in the future, feel free to reopen it! This can be done using the /open pull request command.