8197518: Kerberos krb5 authentication: AuthList's put method leads to performance issue
Reviewed-by: coffeys, xuelei
This commit is contained in:
parent
3e28c2f71d
commit
5d3e5d9275
@ -1,5 +1,5 @@
|
||||
/*
|
||||
* Copyright (c) 2000, 2013, 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 @@ public class AuthList {
|
||||
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 @@ public class AuthList {
|
||||
* 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 @@ public class AuthList {
|
||||
|
||||
// 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() {
|
||||
|
@ -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,9 @@
|
||||
|
||||
package sun.security.krb5.internal.rcache;
|
||||
|
||||
import java.util.*;
|
||||
import java.util.Map;
|
||||
import java.util.concurrent.ConcurrentHashMap;
|
||||
|
||||
import sun.security.krb5.internal.KerberosTime;
|
||||
import sun.security.krb5.internal.KrbApErrException;
|
||||
import sun.security.krb5.internal.ReplayCache;
|
||||
@ -48,31 +50,18 @@ public class MemoryCache extends ReplayCache {
|
||||
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 Map<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);
|
||||
content.computeIfAbsent(key, k -> new AuthList(lifespan))
|
||||
.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() {
|
||||
|
Loading…
Reference in New Issue
Block a user