[15] RFR: 8245241: Incorrect locale provider preference is not logged

[naoto.sato at oracle.com]

Hi Daniel,

Thank you for your review!

On 5/21/20 3:42 AM, Daniel Fuchs wrote:
> Hi Naoto,
> 
> Logging uses:
> 
> ZonedDateTime zdt = ZonedDateTime.ofInstant(
>                  record.getInstant(), ZoneId.systemDefault());
> 
> and then
> 
> String.format("%1$tb %1$td, %1$tY %1$tl:%1$tM:%1$tS %1$Tp %2$s%n%4$s: 
> %5$s%6$s%n", ...)
> 
> to format the date.
> 
> If the locale provider can't be loaded, is that going to cause problem,
> when logging is invoked to log the info/warning?
> 
> In particular:
> java.base/share/classes/sun/util/locale/provider/LocaleProviderAdapter.java
> 
>   197 getLogger(LocaleProviderAdapter.class.getCanonicalName())
>   198                             .log(Logger.Level.INFO, e);
>   199                     adapterInstances.putIfAbsent(type, 
> NONEXISTENT_ADAPTER);
>   200                     if (defaultLocaleProviderAdapter == type) {
>   201                         defaultLocaleProviderAdapter = Type.FALLBACK;
>   202                     }
> 
> Maybe logging should only be invoked *after* lines 199-202?

In fact, this piece of code should not happen as those adapter classes 
are all JDK provided classes. I replaced the above code with 
ServiceConfigurationError, like other similar locations (e.g, 
HostLocaleProviderAdapter - line 64-65). Updated the webrev:

http://cr.openjdk.java.net/~naoto/8245241/webrev.01/

> 
> Same potential issue in
> src/java.base/share/classes/sun/util/locale/provider/LocaleServiceProviderPool.java
> 
> 
> lines 367-372

This should be fine, as LocaleProviderAdapter should properly have been 
initialized at this point.

Naoto

> 
> best regards,
> 
> -- daniel
> 
> On 20/05/2020 18:29, naoto.sato at oracle.com wrote:
>> Hello,
>>
>> Please review the fix to the following issue:
>>
>> https://bugs.openjdk.java.net/browse/JDK-8245241
>>
>> The proposed changeset is located at:
>>
>> https://cr.openjdk.java.net/~naoto/8245241/webrev.00/
>>
>> Incorrect user-provided provider preference is supposed to be logged. 
>> However it is not so because it is using 
>> PlatformLogger(SurrogateLogger) which uses the default logging level. 
>> I changed it to use j.l.System's logger and bumped it to INFO level 
>> (it should notify the user by default). By taking this opportunity, I 
>> did some clean-up in locale provider adapter's logging related code as 
>> well.
>>
>> Naoto
>