On Fri, 10 Apr 2026 18:41:27 GMT, Brent Christian <[email protected]> wrote:

> This is a pull request to convert the finalizer in 
> `sun.security.smartcardio.CardImpl` to use Cleaner instead. The relevant 
> state is refactored into a Context object, in the standard fashion.
> 
> This change uses the recommended `try`/`finally`/`reachabilityFence()` 
> technique to prevent races between the program thread and the Cleaner thread, 
> per the `Reference.reachabilityFence()` API Note:
> 
>> _there is a race between the program thread running the method, and the 
>> cleanup thread running the Cleaner or finalizer. The cleanup thread could 
>> free a resource, followed by the program thread (still running the method) 
>> attempting to access the now-already-freed resource. Use of 
>> reachabilityFence can prevent this race by ensuring that the object remains 
>> strongly reachable._
> 
> See 
> [Reference.reachabilityFence()](https://docs.oracle.com/en/java/javase/26/docs/api/java.base/java/lang/ref/Reference.html#reachabilityFence(java.lang.Object))
>  and [java.lang.ref - Memory 
> Visibility](https://docs.oracle.com/en/java/javase/26/docs/api/java.base/java/lang/ref/package-summary.html#memory-consistency-properties-heading)
>  for details / background info.
> 
> The test creates a `Card` object, and allows it to be collected. This 
> confirms that the new cleaning action does not hold onto the Owner ("this") 
> object, per the 
> [Cleaner](https://docs.oracle.com/en/java/javase/26/docs/api/java.base/java/lang/ref/Cleaner.html)
>  class API Note ("_it is important that the object implementing the cleaning 
> action does not hold references to the object_").
> 
> I've tried to make the test similar to nearby JavaCard tests. I tried it on 
> Windows using the latest JavaCard Simulator.
> 
> ---------
> - [x] I confirm that I make this contribution in accordance with the [OpenJDK 
> Interim AI Policy](https://openjdk.org/legal/ai).

src/java.base/share/classes/module-info.java line 2:

> 1: /*
> 2:  * Copyright (c) 2014, 2025, Oracle and/or its affiliates. All rights 
> reserved.

nit: 
Suggestion:

 * Copyright (c) 2014, 2026, Oracle and/or its affiliates. All rights reserved.

src/java.smartcardio/share/classes/sun/security/smartcardio/ChannelImpl.java 
line 2:

> 1: /*
> 2:  * Copyright (c) 2005, 2021, Oracle and/or its affiliates. All rights 
> reserved.

nit: 
Suggestion:

 * Copyright (c) 2005, 2026, Oracle and/or its affiliates. All rights reserved.

test/jdk/sun/security/smartcardio/TestCleaner.java line 42:

> 40: // This test requires special hardware; a card must be present
> 41: 
> 42: import java.util.List;

List and terminal factory are unused imports as far as I can see. Could you 
please remove them?

test/jdk/sun/security/smartcardio/TestCleaner.java line 54:

> 52:  * CardImpl from becoming unreachable and being collected/cleaned.
> 53:  */
> 54: public class TestCleaner extends Utils {

Since this is a manual test have you considered using a ui for instructions? I 
think the go-to method now is `UIBuilder`. e.g. : 

    final JDialog dialog = new UIBuilder.DialogBuilder()
                .setTitle("Title")
                .setInstruction(instruction)
                .setMessage(message)
                .setPassAction(e -> pass())
                .setFailAction(e -> fail())
                .setCloseAction(this::abort)
                .build();

https://github.com/openjdk/jdk/blob/master/test/jdk/com/sun/security/auth/callback/TextCallbackHandler/Password.java#L85

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

PR Review Comment: https://git.openjdk.org/jdk/pull/30683#discussion_r3085329714
PR Review Comment: https://git.openjdk.org/jdk/pull/30683#discussion_r3085325679
PR Review Comment: https://git.openjdk.org/jdk/pull/30683#discussion_r3085275526
PR Review Comment: https://git.openjdk.org/jdk/pull/30683#discussion_r3085320555

Reply via email to