On Sun, 6 Mar 2022 15:00:47 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
>> src/java.base/share/classes/java/util/Formatter.java line 2012:
>>
>>> 2010: public final class Formatter implements Closeable, Flushable {
>>> 2011: // Caching DecimalFormatSymbols
>>> 2012: static DecimalFormatSymbols DFS = null;
>>
>> Hello Jim,
>> The javadoc of `Formatter` states:
>>
>>>
>>> Formatters are not necessarily safe for multithreaded access. Thread
>>> safety is optional and is the responsibility of users of methods in this
>>> class.
>>>
>>
>> So I would think that user applications would typically be synchronizing on
>> the instance of a `Formatter` for any multi-threaded use of a formatter
>> instance.
>>
>> If I'm reading this changed code correctly, even if user applications have
>> properly synchronized on a `Formatter` instance, it's now possible that 2
>> separate instances of a `Formatter` being concurrently accessed (i.e. in
>> different threads) will now try and update/use this cached class level
>> `static` state `DFS`. That would thus require some kind of thread safety
>> semantics to be implemented for this new `getDecimalFormatSymbols(Locale)`
>> method, isn't it?
>
>> will now try and update/use this cached class level static state DFS. That
>> would thus require some kind of thread safety semantics to be implemented
>> for this new getDecimalFormatSymbols(Locale) method, isn't it?
>
> A bit more closer look at the code and it appears to me that the use of :
>
> DecimalFormatSymbols dfs = DFS;
>
> and then working off that local variable prevents any kind of race issues
> that might be caused due to multi-thread access. Of course it still means
> that multiple threads might still go ahead and do a:
>
> dfs = DecimalFormatSymbols.getInstance(locale);
>
> on first access (when `DFS` is null) but that in itself should be harmless.
>
> If this is intentional (which I suspect it is), should some comment be added
> in this method explaining this multi-thread access detail?
Initially, I thought of the same thing (not the `Formatter` but
`DecimalFormatSymbols` itself, as it is also not thread safe), but I concluded
it was OK, as there is no mutation going on. I agree with Jaikiran that some
comments might help here.
-------------
PR: https://git.openjdk.java.net/jdk/pull/7703
