Mercurial > hg > release > icedtea7-forest-2.6 > jdk
changeset 9879:72426fcae25f
8197518: Kerberos krb5 authentication: AuthList's put method leads to performance issue
Reviewed-by: mbalao
author | andrew |
---|---|
date | Fri, 31 Jan 2020 18:54:01 +0000 |
parents | 04f0514ef0fd |
children | 75b781015ecc |
files | src/share/classes/sun/security/krb5/internal/rcache/AuthList.java src/share/classes/sun/security/krb5/internal/rcache/MemoryCache.java |
diffstat | 2 files changed, 38 insertions(+), 37 deletions(-) [+] |
line wrap: on
line diff
--- a/src/share/classes/sun/security/krb5/internal/rcache/AuthList.java Thu Jan 30 04:12:14 2020 +0000 +++ b/src/share/classes/sun/security/krb5/internal/rcache/AuthList.java Fri Jan 31 18:54:01 2020 +0000 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2000, 2006, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2000, 2018, 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 @@ -55,6 +55,9 @@ private final LinkedList<AuthTimeWithHash> entries; private final int lifespan; + // entries.getLast().ctime, updated after each cleanup. + private volatile int oldestTime = Integer.MIN_VALUE; + /** * Constructs a AuthList. */ @@ -67,11 +70,13 @@ * Puts the authenticator timestamp into the cache in descending order, * and throw an exception if it's already there. */ - public void put(AuthTimeWithHash t, KerberosTime currentTime) + public synchronized void put(AuthTimeWithHash t, KerberosTime currentTime) throws KrbApErrException { if (entries.isEmpty()) { entries.addFirst(t); + oldestTime = t.ctime; + return; } else { AuthTimeWithHash temp = entries.getFirst(); int cmp = temp.compareTo(t); @@ -106,24 +111,26 @@ // let us cleanup while we are here long timeLimit = currentTime.getSeconds() - lifespan; - ListIterator<AuthTimeWithHash> it = entries.listIterator(0); - AuthTimeWithHash temp = null; - int index = -1; - while (it.hasNext()) { - // search expired timestamps. - temp = it.next(); - if (temp.ctime < timeLimit) { - index = entries.indexOf(temp); - break; + + // Only trigger a cleanup when the earliest entry is + // lifespan + 5 sec ago. This ensures a cleanup is done + // at most every 5 seconds so that we don't always + // addLast(removeLast). + if (oldestTime > timeLimit - 5) { + return; + } + + // and we remove the *enough* old ones (1 lifetime ago) + while (!entries.isEmpty()) { + AuthTimeWithHash removed = entries.removeLast(); + if (removed.ctime >= timeLimit) { + entries.addLast(removed); + oldestTime = removed.ctime; + return; } } - // It would be nice if LinkedList has a method called truncate(index). - if (index > -1) { - do { - // remove expired timestamps from the list. - entries.removeLast(); - } while(entries.size() > index); - } + + oldestTime = Integer.MIN_VALUE; } public boolean isEmpty() {
--- a/src/share/classes/sun/security/krb5/internal/rcache/MemoryCache.java Thu Jan 30 04:12:14 2020 +0000 +++ b/src/share/classes/sun/security/krb5/internal/rcache/MemoryCache.java Fri Jan 31 18:54:01 2020 +0000 @@ -1,5 +1,5 @@ /* - * Copyright (c) 2013, Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2013, 2018, 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 @@ -31,7 +31,10 @@ package sun.security.krb5.internal.rcache; -import java.util.*; +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; + import sun.security.krb5.internal.KerberosTime; import sun.security.krb5.internal.KrbApErrException; import sun.security.krb5.internal.ReplayCache; @@ -49,31 +52,22 @@ private static final int lifespan = KerberosTime.getDefaultSkew(); private static final boolean DEBUG = sun.security.krb5.internal.Krb5.DEBUG; - private final Map<String,AuthList> content = new HashMap<>(); + private final ConcurrentMap<String,AuthList> content = new ConcurrentHashMap<>(); @Override public synchronized void checkAndStore(KerberosTime currTime, AuthTimeWithHash time) throws KrbApErrException { String key = time.client + "|" + time.server; - AuthList rc = content.get(key); + AuthList newAuthList = new AuthList(lifespan); + AuthList authList = content.putIfAbsent(key, newAuthList); + if (authList == null) { + authList = newAuthList; + } + authList.put(time, currTime); if (DEBUG) { System.out.println("MemoryCache: add " + time + " to " + key); } - if (rc == null) { - rc = new AuthList(lifespan); - rc.put(time, currTime); - if (!rc.isEmpty()) { - content.put(key, rc); - } - } else { - if (DEBUG) { - System.out.println("MemoryCache: Existing AuthList:\n" + rc); - } - rc.put(time, currTime); - if (rc.isEmpty()) { - content.remove(key); - } - } + // TODO: clean up AuthList entries with only expired AuthTimeWithHash objects. } public String toString() {