RFR: 8284910: Buffer clean in PasswordCallback [v2]

[Xue-Lei Andrew Fan]

On Wed, 20 Apr 2022 23:38:59 GMT, Stuart Marks <smarks at openjdk.org> wrote:

>> Calling `Cleanable.clean()` calls the `Runnable` action at-most-once.
>> Each `Cleanable` inserted in a list when it is created and is removed when `clear()` or `clean()` is invoked.
>> The action is called only if it is still in the list. So there is no extra overhead.
>> 
>> There is no harm in clearing the cleanable field; but it does not save much.
>> 
>> The code in `clearPassword` can be simplified and only test `cleanable != null`; it will be null unless there is an inputPassword to clean.
>
> I'd recommend setting `cleanable` to null after it's been cleaned to make the state machine easier to reason about. The invariant would be: if `cleanable` is non-null, then we have something dirty that needs to be cleaned. If we don't clear it to null after cleaning, it potentially results in confusing states. For example, suppose the app calls `setPassword(nonNull)` and later calls `setPassword(null)`. The second call will set `inputPassword` to null but leave a stale reference in `cleanable`. This isn't necessarily harmful, but it's confusing.

> The code in `clearPassword` can be simplified and only test `cleanable != null`; it will be null unless there is an inputPassword to clean.

Yes.  The testing of `cleanable != null` is sufficient.

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

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