On Wed, 20 Apr 2022 23:38:59 GMT, Stuart Marks <sma...@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
