On Tue, 12 May 2026 18:23:38 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). > > Brent Christian has updated the pull request with a new target base due to a > merge or a rebase. The incremental webrev excludes the unrelated changes > brought in by the merge/rebase. The pull request contains nine additional > commits since the last revision: > > - Merge branch 'master' into smartcardio > - update copyright year > - Merge branch 'master' into smartcardio > - update example code in javax.smartcardio package doc to use try-finally > - minor comment update > - add missed clean() calls, update comments > - fix imports and copyright years > - add test case > - convert CardImpl finalizer to use Cleaner Did another review pass over this PR, after comments are answered/possibly addressed, this looks good to go from me src/java.smartcardio/share/classes/sun/security/smartcardio/CardImpl.java line 255: > 253: try { > 254: checkSecurity("exclusive"); > 255: checkState(); What's the reason that these checks are done within the try-finally whereas the `openLogicalChannel()`-checks are done outside of the try-finally? src/java.smartcardio/share/classes/sun/security/smartcardio/CardImpl.java line 275: > 273: public synchronized void endExclusive() throws CardException { > 274: try { > 275: checkState(); Same comment as https://github.com/openjdk/jdk/pull/30683/changes#r3280643716 src/java.smartcardio/share/classes/sun/security/smartcardio/CardImpl.java line 298: > 296: checkSecurity("transmitControl"); > 297: checkState(); > 298: checkExclusive(); Same comment as https://github.com/openjdk/jdk/pull/30683/changes#r3280643716 src/java.smartcardio/share/classes/sun/security/smartcardio/CardImpl.java line 322: > 320: return; > 321: } > 322: checkExclusive(); Same comment as https://github.com/openjdk/jdk/pull/30683/changes#r3280643716 ------------- PR Review: https://git.openjdk.org/jdk/pull/30683#pullrequestreview-4336329957 PR Review Comment: https://git.openjdk.org/jdk/pull/30683#discussion_r3280643716 PR Review Comment: https://git.openjdk.org/jdk/pull/30683#discussion_r3280645592 PR Review Comment: https://git.openjdk.org/jdk/pull/30683#discussion_r3280646512 PR Review Comment: https://git.openjdk.org/jdk/pull/30683#discussion_r3280648997
