[crac] RFR: CRaC related documentation in JDK classes using custom tag [v2]

Anton Kozlov akozlov at openjdk.org
Thu Apr 27 17:29:54 UTC 2023


On Tue, 18 Apr 2023 16:06:58 GMT, Radim Vansa <duke at openjdk.org> wrote:

>> src/java.base/share/classes/java/lang/System.java line 793:
>> 
>>> 791:      * a resource and in the {@link javax.crac.Resource#afterRestore(javax.crac.Context) afterRestore method}
>>> 792:      * reload system properties, propagating any change.
>>> 793:      *
>> 
>> The comment above is about standard system properties.
>> 
>> Here we should say that system properties are updated after restore. The app can check the updated value in the afterRestore.
>
> Isn't that exactly what is written in the comment?

Sorry, missed the comment. To highlight, I read the the it's being discouraged to change _standard_ system properties, which may be cached during the JDK implementation. Users can chose another set of assumptions, and with CRaC, they'd likely want for properties to be updated (to get more input from user, for more flexibile programs). So we don't want to color the statement and just want to inform the system properties are updated. 
And in the second statement, I propose change "should" -> "can" at least, documenting the possibilty rather than obligation.

>> src/java.base/share/classes/java/security/SecureRandom.java line 366:
>> 
>>> 364:      * the {@link Security#getProviders() Security.getProviders()} method.
>>> 365:      *
>>> 366:      * @crac See provider documentation for details of behaviour after restore from a checkpoint.
>> 
>> Shouldn't be a link here?
>
> You mean to the generic provider interface? Here I meant the implementation of the provider.

I see, I was thinking you were refering something concrete, in that case it's better to have a `{@link ...}` to that. But it looks like "the checkpoint/restore behavior of the instance depends on the particular implementation". Is that correct? If so, worth to rephrase.

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

PR Review Comment: https://git.openjdk.org/crac/pull/51#discussion_r1179472639
PR Review Comment: https://git.openjdk.org/crac/pull/51#discussion_r1179484540


More information about the crac-dev mailing list