RFR: 8284161: Implementation of Virtual Threads (Preview) [v3]

[Alan Bateman]

On Sun, 17 Apr 2022 03:49:19 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>> Alan Bateman has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains five additional commits since the last revision:
>> 
>>  - Refresh
>>  - Refresh
>>  - Merge with jdk-19+18
>>  - Refresh
>>  - Initial push
>
> src/java.base/share/classes/java/io/PrintStream.java line 1420:
> 
>> 1418:             } else {
>> 1419:                 synchronized (this) {
>> 1420:                     implFormat(format, args);
> 
> I think there's a typo here. I think it should have been:
> 
> 
> implFormat(l, format, args);

Well spotted, the locale should be provided.

> src/java.base/share/classes/java/lang/Thread.java line 602:
> 
>> 600:         } else {
>> 601:             // getContextClassLoader not trusted
>> 602:             ClassLoader cl = parent.contextClassLoader;
> 
> I might be misreading the comment on that line, but reading this if/else block a few times, I'm unsure what the expectations here are.
> 
> It's my understanding that a call to `getContextClassLoader()` can't be trusted if that method has been overridden by the passed `Thread` type. In such cases, we don't call that method and instead just use the field value of `contextClassLoader` (which is a private field on `Thread`). Is that understanding correct? If yes, then the condition in the `if` block a few lines above, looks odd. It seems to be calling the `getContextClassLoader()` if  that method is overridden by the passed `Thread` type, i.e. the untrusted case. Should it instead be:
> 
> if (sm == null || !isCCLOverridden(parent.getClass())) {
>     return parent.getContextClassLoader();
> }
> 
> (notice the negation)

This code hasn't changed, it's just moved to a helper method. When running with a security manager and the CCL methods aren't overridden, then it avoids the permission check. However, I can see how the comment is mis-reading so we can improve that.

-------------

PR: https://git.openjdk.java.net/jdk/pull/8166