On Tue, 13 Apr 2021 02:34:15 GMT, Joe Wang <jo...@openjdk.org> wrote:

>> Naoto Sato has updated the pull request incrementally with one additional 
>> commit since the last revision:
>> 
>>   Reverted PrintStream changes
>
> src/java.base/share/classes/java/lang/System.java line 2020:
> 
>> 2018:         setIn0(new BufferedInputStream(fdIn));
>> 2019:         setOut0(newPrintStream(fdOut, cs));
>> 2020:         setErr0(newPrintStream(fdErr, cs));
> 
> It was getting from sun.stdout.encoding or sun.stderr.encoding. After the 
> change, when there is no console, it would be set with 
> Charset.defaultCharset(). Would that be an incompatible change?

Although the code path is different, the logic to determine the encoding is not 
changed, as `sun.stdout/err.encoding` are only set if the VM is invoked from a 
terminal (in fact, there's a bug where they aren't set even in a terminal on 
unix env, which I fixed in this patch) which is the same condition in which 
`System.console()` returns the console. And for both cases, it will default to 
`Charset.defaultCharset()` if they are not available.

Having said that, one regression test started to fail with this implementation 
change as follows:

Exception in thread "main" java.util.ServiceConfigurationError: Locale provider 
adapter "CLDR"cannot be instantiated.
        at 
java.base/sun.util.locale.provider.LocaleProviderAdapter.forType(LocaleProviderAdapter.java:199)
        at 
java.base/sun.util.locale.provider.LocaleProviderAdapter.findAdapter(LocaleProviderAdapter.java:287)
        at 
java.base/sun.util.locale.provider.LocaleProviderAdapter.getAdapter(LocaleProviderAdapter.java:258)
        at java.base/java.text.NumberFormat.getInstance(NumberFormat.java:960)
        at 
java.base/java.text.NumberFormat.getNumberInstance(NumberFormat.java:518)
        at java.base/java.util.Scanner.useLocale(Scanner.java:1270)
        at java.base/java.util.Scanner.<init>(Scanner.java:543)
        at java.base/java.util.Scanner.<init>(Scanner.java:554)
        at TestConsole.main(TestConsole.java:54)
Caused by: java.lang.reflect.InvocationTargetException
        at 
java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance0(Native
 Method)
        at 
java.base/jdk.internal.reflect.NativeConstructorAccessorImpl.newInstance(NativeConstructorAccessorImpl.java:78)
        at 
java.base/jdk.internal.reflect.DelegatingConstructorAccessorImpl.newInstance(DelegatingConstructorAccessorImpl.java:45)
        at 
java.base/java.lang.reflect.Constructor.newInstanceWithCaller(Constructor.java:499)
        at 
java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:480)
        at 
java.base/sun.util.locale.provider.LocaleProviderAdapter.forType(LocaleProviderAdapter.java:188)
        ... 8 more

I haven't looked at it in detail but it seems that calling `System.console()` 
in `System.initPhase1()` is not allowed, as the module system is not there yet. 
So I reverted the implementation to the original and simply retained the 
description in `System.out/err` with a change to `determined by` to `equivalent 
to`.

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

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

Reply via email to