RFR: 8240256: Better resource cleaning for SunPKCS11 Provider

[Valerie Peng]

On Fri, 16 Apr 2021 11:24:57 GMT, Sean Coffey <coffeys at openjdk.org> wrote:

> Added capability to allow the PKCS11 Token to be destroyed once a session is logged out from. New configuration properties via pkcs11 config file. Cleaned up the native resource poller also.
> 
> New unit test case to test behaviour. Some PKCS11 tests refactored to allow pkcs11 provider to be configured (and tested) with a config file of choice.
> 
> Reviewer request @valeriepeng

Here are some comments. Mostly just nit. Will continue looking at the test changes. 
Thanks~ 
Valerie

src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/Config.java line 434:

> 432:                     throw excLine(word + " must be at least 100 ms");
> 433:                 }
> 434:             } else if (word.endsWith("cleaner.shortInterval")) {

Why use endsWith()? Most of other parsing code are enforcing equality, i.e. equals()?

src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/KeyCache.java line 2:

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

2021? Same goes for other files.

src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/P11Key.java line 176:

> 174:             found = true;
> 175:             next.dispose();
> 176:         }

nit: maybe this can be shortened a little with:
SessionKeyRef next;
while ((next = (SessionKeyRef) SessionKeyRef.refQueue.poll()) != null) {
    found = true;
    next.dispose();
}

Same goes for the other drainRefQueue() impl.

src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/Session.java line 133:

> 131:     }
> 132: 
> 133:     static boolean drainRefQueue() {

Add a comment on this being called by the cleaner thread with the two interval configuration options? Sames goes for the one in P11Key.java.

src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/Session.java line 152:

> 150:         implements Comparable<SessionRef> {
> 151: 
> 152:     static ReferenceQueue<Session> refQueue =

nit: can now move the value assignment onto the same line. Same goes for the refQueue in P11Key.java.

src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/SessionManager.java line 246:

> 244:         private final AbstractQueue<Session> pool;
> 245:         private final int SESSION_MAX = 5;
> 246:         // access is synchronized on 'this'

Maybe old comment?

src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/SunPKCS11.java line 963:

> 961:         private long sleepMillis = config.getResourceCleanerShortInterval();
> 962:         private int count = 0;
> 963:         boolean p11RefFound, SessRefFound;

nit: p11RefFound => keyRefFound?
nit: SessRefFound => sessRefFound?

src/jdk.crypto.cryptoki/share/classes/sun/security/pkcs11/SunPKCS11.java line 992:

> 990:                     // increase the sleep time
> 991:                     sleepMillis = config.getResourceCleanerLongInterval();
> 992:                 }

This if-check can be moved up to below the line "count++;"?

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

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