8098580: drainRefQueueBounds() puts pressure on pool.size()
authorascarpino
Tue, 05 Apr 2016 11:48:30 -0700
changeset 36927 b59fed2c77e2
parent 36926 c49df7b25461
child 36928 71f5c0728dfc
8098580: drainRefQueueBounds() puts pressure on pool.size() Reviewed-by: valeriep
jdk/src/jdk.crypto.pkcs11/share/classes/sun/security/pkcs11/P11Key.java
jdk/src/jdk.crypto.pkcs11/share/classes/sun/security/pkcs11/SessionManager.java
--- a/jdk/src/jdk.crypto.pkcs11/share/classes/sun/security/pkcs11/P11Key.java	Tue Apr 05 09:13:52 2016 -0700
+++ b/jdk/src/jdk.crypto.pkcs11/share/classes/sun/security/pkcs11/P11Key.java	Tue Apr 05 11:48:30 2016 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2015, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2016, 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
@@ -45,6 +45,7 @@
 import sun.security.pkcs11.wrapper.*;
 import static sun.security.pkcs11.wrapper.PKCS11Constants.*;
 
+import sun.security.util.Debug;
 import sun.security.util.DerValue;
 import sun.security.util.Length;
 import sun.security.util.ECUtil;
@@ -1110,11 +1111,41 @@
     }
 
     private static void drainRefQueueBounded() {
+        Session sess = null;
+        Token tkn = null;
         while (true) {
             SessionKeyRef next = (SessionKeyRef) refQueue.poll();
-            if (next == null) break;
+            if (next == null) {
+                break;
+            }
+
+            // If the token is still valid, try to remove the object
+            if (next.session.token.isValid()) {
+                // If this key's token is the same as the previous key, the
+                // same session can be used for C_DestroyObject.
+                try {
+                    if (next.session.token != tkn || sess == null) {
+                        // Release session if not using previous token
+                        if (tkn != null && sess != null) {
+                            tkn.releaseSession(sess);
+                            sess = null;
+                        }
+
+                        tkn = next.session.token;
+                        sess = tkn.getOpSession();
+                    }
+                    next.disposeNative(sess);
+                } catch (PKCS11Exception e) {
+                    // ignore
+                }
+            }
+            // Regardless of native results, dispose of java references
             next.dispose();
         }
+
+        if (tkn != null && sess != null) {
+            tkn.releaseSession(sess);
+        }
     }
 
     // handle to the native key
@@ -1127,25 +1158,17 @@
         this.session = session;
         this.session.addObject();
         refList.add(this);
-        // TBD: run at some interval and not every time?
         drainRefQueueBounded();
     }
 
+    private void disposeNative(Session s) throws PKCS11Exception {
+        session.token.p11.C_DestroyObject(s.id(), keyID);
+    }
+
     private void dispose() {
         refList.remove(this);
-        if (session.token.isValid()) {
-            Session newSession = null;
-            try {
-                newSession = session.token.getOpSession();
-                session.token.p11.C_DestroyObject(newSession.id(), keyID);
-            } catch (PKCS11Exception e) {
-                // ignore
-            } finally {
-                this.clear();
-                session.token.releaseSession(newSession);
-                session.removeObject();
-            }
-        }
+        this.clear();
+        session.removeObject();
     }
 
     public int compareTo(SessionKeyRef other) {
--- a/jdk/src/jdk.crypto.pkcs11/share/classes/sun/security/pkcs11/SessionManager.java	Tue Apr 05 09:13:52 2016 -0700
+++ b/jdk/src/jdk.crypto.pkcs11/share/classes/sun/security/pkcs11/SessionManager.java	Tue Apr 05 11:48:30 2016 -0700
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2003, 2014, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2003, 2016, 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
@@ -34,7 +34,7 @@
 import sun.security.pkcs11.wrapper.*;
 import static sun.security.pkcs11.wrapper.PKCS11Constants.*;
 
-import java.util.concurrent.ConcurrentLinkedDeque;
+import java.util.concurrent.LinkedBlockingQueue;
 import java.util.concurrent.atomic.AtomicInteger;
 
 /**
@@ -112,8 +112,8 @@
         }
         maxSessions = (int)Math.min(n, Integer.MAX_VALUE);
         this.token = token;
-        this.objSessions = new Pool(this);
-        this.opSessions = new Pool(this);
+        this.objSessions = new Pool(this, true);
+        this.opSessions = new Pool(this, false);
         if (debug != null) {
             maxActiveSessionsLock = new Object();
         }
@@ -236,12 +236,18 @@
     public static final class Pool {
 
         private final SessionManager mgr;
-
-        private final ConcurrentLinkedDeque<Session> pool;
+        private final AbstractQueue<Session> pool;
+        private final int SESSION_MAX = 5;
 
-        Pool(SessionManager mgr) {
-           this.mgr = mgr;
-           pool = new ConcurrentLinkedDeque<Session>();
+        // Object session pools can contain unlimited sessions.
+        // Operation session pools are limited and enforced by the queue.
+        Pool(SessionManager mgr, boolean obj) {
+            this.mgr = mgr;
+            if (obj) {
+                pool = new LinkedBlockingQueue<Session>();
+            } else {
+                pool = new LinkedBlockingQueue<Session>(SESSION_MAX);
+            }
         }
 
         boolean remove(Session session) {
@@ -249,24 +255,24 @@
         }
 
         Session poll() {
-            return pool.pollLast();
+            return pool.poll();
         }
 
         void release(Session session) {
-            pool.offer(session);
-            if (session.hasObjects()) {
-                return;
+            // Object session pools never return false, only Operation ones
+            if (!pool.offer(session)) {
+                mgr.closeSession(session);
+                free();
             }
+        }
 
-            int n = pool.size();
-            if (n < 5) {
-                return;
-            }
-
+        // Free any old operation session if this queue is full
+        void free() {
+            int n = SESSION_MAX;
+            int i = 0;
             Session oldestSession;
             long time = System.currentTimeMillis();
-            int i = 0;
-            // Check if the session head is too old and continue through queue
+            // Check if the session head is too old and continue through pool
             // until only one is left.
             do {
                 oldestSession = pool.peek();