Hi, Tony,
<SessionManager.java>
- line 243, 'be' should be "by"?
- line 271, why use SESSION_MAX - 1 instead of just SESSION_MAX? The
check on line 286 make sure that the queue will not be empty. Actually,
the comment has "until only one is left", but the condition is ">1", so
shouldn't it be "until 2 left"? This is not what u changed, just noticed
it and wonder if I read the code wrong.
<P11Key.java>
- line 1114 and 1115, can we rename these 2 variables? The terms session
and token are all over this SessionKeyRef class. Some are local, some
are member fields. It'd make the code much more easier to follow if they
are named something less common, say "sess" or "tok".
- line 1126 - 1127, it's clearer to have {} to clearly separate the code.
- After the while-true loop (line 1116 - 1134), don't you have to call
token.releaseSession(session) again? Otherwise, the most recently
allocated op session seems to not released back to the pool.
- There was a session.token.isValid() check in the older dispose()
method but not in the new one. Shouldn't we still check before getting a
new op session?
- The two dispose methods do entirely different things, it'd be better
to name them differently.
Thanks,
Valerie
On 3/1/2016 2:29 PM, Anthony Scarpino wrote:
Hi,
I need a review of this change to SunPKCS11 where it's a bit smarter
in disposing of native library object by reusing the same pkcs11
session is possible. Additionally changes to the session pool to fix
a performance bottleneck.
http://cr.openjdk.java.net/~ascarpino/8098580/webrev/
thanks
Tony
