Skip to content

8380391: Update java.smartcardio finalizer to use Cleaner #30683

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bchristi-git wants to merge 2 commits into
base: master
Choose a base branch
from
Open

8380391: Update java.smartcardio finalizer to use Cleaner #30683

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ee4339f
convert CardImpl finalizer to use Cleaner
bchristi-git Apr 2, 2026
48ca793
add test case
bchristi-git Apr 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/java.base/share/classes/module-info.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,7 @@
exports jdk.internal.ref to
java.desktop,
java.net.http,
java.smartcardio,
jdk.naming.dns;
exports jdk.internal.reflect to
java.logging,
Expand Down
249 changes: 154 additions & 95 deletions src/java.smartcardio/share/classes/sun/security/smartcardio/CardImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2005, 2023, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2005, 2026, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand All @@ -26,6 +26,9 @@
package sun.security.smartcardio;

import jdk.internal.util.OperatingSystem;
import jdk.internal.ref.CleanerFactory;
import java.lang.ref.Cleaner;
import java.lang.ref.Reference;

import javax.smartcardio.*;
import static sun.security.smartcardio.PCSC.*;
Expand All @@ -43,9 +46,6 @@ private static enum State { OK, REMOVED, DISCONNECTED };
// the terminal that created this card
private final TerminalImpl terminal;

// the native SCARDHANDLE
final long cardId;

// atr of this card
private final ATR atr;

Expand All @@ -55,12 +55,40 @@ private static enum State { OK, REMOVED, DISCONNECTED };
// the basic logical channel (channel 0)
private final ChannelImpl basicChannel;

// state of this card connection
private volatile State state;

// thread holding exclusive access to the card, or null
private volatile Thread exclusiveThread;

/* State and code for cleanup */
static class Context implements Runnable {
// the native SCARDHANDLE
final long cardId;

// state of this card connection
private volatile State state;

private Context(long cardId, State state) {
this.cardId = cardId;
this.state = state;
}

public void run() {
if (state == State.OK) {
state = State.DISCONNECTED;
try {
SCardDisconnect(cardId, SCARD_LEAVE_CARD);
} catch (PCSCException e) {
// This will be swallowed if thrown when run by the Cleaner
// thread, and never thrown if called via Cleanable.clean()
throw new RuntimeException(e);
}
}
}
}

final Context context;
private final Cleaner.Cleanable cleanable;


CardImpl(TerminalImpl terminal, String protocol) throws PCSCException {
this.terminal = terminal;
int sharingMode = SCARD_SHARE_SHARED;
Expand All @@ -83,36 +111,48 @@ private static enum State { OK, REMOVED, DISCONNECTED };
} else {
throw new IllegalArgumentException("Unsupported protocol " + protocol);
}
cardId = SCardConnect(terminal.contextId, terminal.name,
long localCardId = SCardConnect(terminal.contextId, terminal.name,
sharingMode, connectProtocol);
byte[] status = new byte[2];
byte[] atrBytes = SCardStatus(cardId, status);
byte[] atrBytes = SCardStatus(localCardId, status);
atr = new ATR(atrBytes);
this.protocol = status[1] & 0xff;
basicChannel = new ChannelImpl(this, 0);
state = State.OK;

this.context = new Context(localCardId, State.OK);
this.cleanable = CleanerFactory.cleaner().register(this, this.context);
}

void checkState() {
State s = state;
if (s == State.DISCONNECTED) {
throw new IllegalStateException("Card has been disconnected");
} else if (s == State.REMOVED) {
throw new IllegalStateException("Card has been removed");
try {
State s = context.state;
if (s == State.DISCONNECTED) {
throw new IllegalStateException("Card has been disconnected");
} else if (s == State.REMOVED) {
throw new IllegalStateException("Card has been removed");
}
} finally {
Reference.reachabilityFence(this);
}
}

boolean isValid() {
if (state != State.OK) {
return false;
}
// ping card via SCardStatus
try {
SCardStatus(cardId, new byte[2]);
return true;
} catch (PCSCException e) {
state = State.REMOVED;
return false;
if (context.state != State.OK) {
return false;
}
// ping card via SCardStatus
try {
SCardStatus(context.cardId, new byte[2]);
return true;
} catch (PCSCException e) {
context.state = State.REMOVED;
// Cleaner no longer needs to track
cleanable.clean(); // FIXME: why?
Copy link
Copy Markdown

@Marcono1234 Marcono1234 Apr 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this FIXME still open?

Though it seems to be the same behavior as before; there it also set state = State.REMOVED, making finalize() effectively nop.
So is this mostly a "why was this done like this before"?

return false;
}
} finally {
Reference.reachabilityFence(this);
}
}

Expand All @@ -125,8 +165,12 @@ private void checkSecurity(String action) {
}

void handleError(PCSCException e) {
if (e.code == SCARD_W_REMOVED_CARD) {
state = State.REMOVED;
try {
if (e.code == SCARD_W_REMOVED_CARD) {
context.state = State.REMOVED;
}
} finally {
Reference.reachabilityFence(this);
}
}

Expand Down Expand Up @@ -164,21 +208,25 @@ private static int getSW(byte[] b) {
private static byte[] commandOpenChannel = new byte[] {0, 0x70, 0, 0, 1};

public CardChannel openLogicalChannel() throws CardException {
checkSecurity("openLogicalChannel");
checkState();
checkExclusive();
try {
byte[] response = SCardTransmit
(cardId, protocol, commandOpenChannel, 0, commandOpenChannel.length);
if ((response.length != 3) || (getSW(response) != 0x9000)) {
throw new CardException
("openLogicalChannel() failed, card response: "
+ PCSC.toString(response));
checkSecurity("openLogicalChannel");
checkState();
checkExclusive();
try {
byte[] response = SCardTransmit
(context.cardId, protocol, commandOpenChannel, 0, commandOpenChannel.length);
if ((response.length != 3) || (getSW(response) != 0x9000)) {
throw new CardException
("openLogicalChannel() failed, card response: "
+ PCSC.toString(response));
}
return new ChannelImpl(this, response[0]);
} catch (PCSCException e) {
handleError(e);
throw new CardException("openLogicalChannel() failed", e);
}
return new ChannelImpl(this, response[0]);
} catch (PCSCException e) {
handleError(e);
throw new CardException("openLogicalChannel() failed", e);
} finally {
Reference.reachabilityFence(this);
}
}

Expand All @@ -193,87 +241,98 @@ void checkExclusive() throws CardException {
}

public synchronized void beginExclusive() throws CardException {
checkSecurity("exclusive");
checkState();
if (exclusiveThread != null) {
throw new CardException
("Exclusive access has already been assigned to Thread "
+ exclusiveThread.getName());
}
try {
SCardBeginTransaction(cardId);
} catch (PCSCException e) {
handleError(e);
throw new CardException("beginExclusive() failed", e);
checkSecurity("exclusive");
checkState();
if (exclusiveThread != null) {
throw new CardException
("Exclusive access has already been assigned to Thread "
+ exclusiveThread.getName());
}
try {
SCardBeginTransaction(context.cardId);
} catch (PCSCException e) {
handleError(e);
throw new CardException("beginExclusive() failed", e);
}
exclusiveThread = Thread.currentThread();
} finally {
Reference.reachabilityFence(this);
}
exclusiveThread = Thread.currentThread();
}

public synchronized void endExclusive() throws CardException {
checkState();
if (exclusiveThread != Thread.currentThread()) {
throw new IllegalStateException
("Exclusive access not assigned to current Thread");
}
try {
SCardEndTransaction(cardId, SCARD_LEAVE_CARD);
} catch (PCSCException e) {
handleError(e);
throw new CardException("endExclusive() failed", e);
checkState();
if (exclusiveThread != Thread.currentThread()) {
throw new IllegalStateException
("Exclusive access not assigned to current Thread");
}
try {
SCardEndTransaction(context.cardId, SCARD_LEAVE_CARD);
} catch (PCSCException e) {
handleError(e);
throw new CardException("endExclusive() failed", e);
} finally {
exclusiveThread = null;
}
} finally {
exclusiveThread = null;
Reference.reachabilityFence(this);
}
}

public byte[] transmitControlCommand(int controlCode, byte[] command)
throws CardException {
checkSecurity("transmitControl");
checkState();
checkExclusive();
if (command == null) {
throw new NullPointerException();
}
try {
byte[] r = SCardControl(cardId, controlCode, command);
return r;
} catch (PCSCException e) {
handleError(e);
throw new CardException("transmitControlCommand() failed", e);
checkSecurity("transmitControl");
checkState();
checkExclusive();
if (command == null) {
throw new NullPointerException();
}
try {
byte[] r = SCardControl(context.cardId, controlCode, command);
return r;
} catch (PCSCException e) {
handleError(e);
throw new CardException("transmitControlCommand() failed", e);
}
} finally {
Reference.reachabilityFence(this);
}
}

public void disconnect(boolean reset) throws CardException {
if (reset) {
checkSecurity("reset");
}
if (state != State.OK) {
return;
}
checkExclusive();
try {
SCardDisconnect(cardId, (reset ? SCARD_RESET_CARD : SCARD_LEAVE_CARD));
} catch (PCSCException e) {
throw new CardException("disconnect() failed", e);
if (reset) {
checkSecurity("reset");
}
if (context.state != State.OK) { // Should this also de-register cleaner?
return;
}
checkExclusive();
try {
SCardDisconnect(context.cardId, (reset ? SCARD_RESET_CARD : SCARD_LEAVE_CARD));
} catch (PCSCException e) {
throw new CardException("disconnect() failed", e);
} finally {
context.state = State.DISCONNECTED;
exclusiveThread = null;
// Unregister from Cleaner.
// State now set to DISCONNECTED, so cleaning action will do nothing
cleanable.clean();
}
} finally {
state = State.DISCONNECTED;
exclusiveThread = null;
Reference.reachabilityFence(this);
}
}

public String toString() {
return "PC/SC card in " + terminal.name
+ ", protocol " + getProtocol() + ", state " + state;
}

@SuppressWarnings("removal")
protected void finalize() throws Throwable {
try {
if (state == State.OK) {
state = State.DISCONNECTED;
SCardDisconnect(cardId, SCARD_LEAVE_CARD);
}
return "PC/SC card in " + terminal.name
+ ", protocol " + getProtocol() + ", state " + context.state;
} finally {
super.finalize();
Reference.reachabilityFence(this);
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/java.smartcardio/share/classes/sun/security/smartcardio/ChannelImpl.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ private byte[] doTransmit(byte[] command) throws CardException {
" exceeded maximum " + RESPONSE_ITERATIONS);
}
byte[] response = SCardTransmit
(card.cardId, card.protocol, command, 0, n);
(card.context.cardId, card.protocol, command, 0, n);
int rn = response.length;
if (getresponse && (rn >= 2) && (n >= 1)) {
// see ISO 7816/2005, 5.1.3
Expand Down Expand Up @@ -280,7 +280,7 @@ public void close() throws CardException {
byte[] com = new byte[] {0x00, 0x70, (byte)0x80, 0};
com[3] = (byte)getChannelNumber();
setChannel(com);
byte[] res = SCardTransmit(card.cardId, card.protocol, com, 0, com.length);
byte[] res = SCardTransmit(card.context.cardId, card.protocol, com, 0, com.length);
if (isOK(res) == false) {
throw new CardException("close() failed: " + PCSC.toString(res));
}
Expand Down
Loading