On Wed, 4 Jan 2023 14:37:13 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
> Can I please get a review of this doc only change which addresses the javadoc > issue noted in https://bugs.openjdk.org/browse/JDK-8258776? > > As noted in that issue, the `ThreadLocal.initialValue()` API javadoc suggests > subclassing `ThreadLocal` and overriding the `initialValue()` method instead > of recommending the `withInitial` method that was added in Java 8. > > The commit here updates the javadoc of `initialValue()` method to note that > `withInitial` method is available for use if the programmer wants to provide > an initial value. The updated javadoc continues to note that the > `ThreadLocal` can be subclassed and the method overriden as the other way of > providing an initial value. This change now uses the `@implSpec` construct to > state this default behaviour of the `initialValue()` method. > > Additionally, the `get()` method's javadoc has also been updated to account > for the semantics of `initialValue()`. In fact, in its current form, before > the changes in this PR, the `get()` method states: > >> If the current thread does not support thread locals then >> this method returns its {@link #initialValue} (or {@code null} >> if the {@code initialValue} method is not overridden). > > which isn't accurate, since the `get()` will return a non-null value if the > ThreadLocal was constructed through `ThreadLocal.withInitial`. Here's a > trivial code which contradicts this current javadoc: > > > public class Test { > public static void main(final String[] args) throws Exception { > Thread.ofPlatform().name("hello").allowSetThreadLocals(false).start(() -> > { > var t = ThreadLocal.withInitial(() -> 2); > System.out.println("Thread local value is " + t.get()); > }); > } > } > > Running this with `java --enable-preview --source 21 Test.java` returns: > > Thread local value is 2 > > > The updated javadoc of `get()` in this PR now matches the behaviour of this > method. I believe this will need a CSR, which I'll open once we have an > agreement on these changes. > > Furthermore, the class level javadoc of `ThreadLocal` has an example of > providing an initial value which uses the subclassing strategy. I haven't > updated that part. Should we update that to show the `withInitialValue` usage > instead (and maybe convert it to a snippet)? This pull request has now been integrated. Changeset: e209693a Author: Jaikiran Pai <j...@openjdk.org> URL: https://git.openjdk.org/jdk/commit/e209693a37e49ba5fd5b1ad3404e9dd807c545f3 Stats: 10 lines in 1 file changed: 3 ins; 1 del; 6 mod 8258776: ThreadLocal#initialValue() Javadoc is unaware of ThreadLocal#withInitial() Reviewed-by: alanb ------------- PR: https://git.openjdk.org/jdk/pull/11846
