On Tue, 27 Apr 2021 16:13:31 GMT, Xue-Lei Andrew Fan <xue...@openjdk.org> wrote:
>> Hello All, >> >> Could you please review the fix for the JDK-8241248? >> The issue happens during the TLSv1.3 handshake without server stateless >> session resumption in case of server receives several parallel requests with >> the same pre_shared_key. >> The main idea of the fix is to remove resuming session from the session >> cache in the early stage. >> >> JBS: https://bugs.openjdk.java.net/browse/JDK-8241248 >> Webrev: http://cr.openjdk.java.net/~abakhtin/8241248/webrev.v0/ >> >> The test from the bug report using OpenSSL is passed ( >> -Djdk.tls.server.enableSessionTicketExtension=false ) >> javax/net/ssl and sun/security/ssl jtreg tests passed >> >> Regards >> Alexey > > src/java.base/share/classes/sun/security/ssl/PreSharedKeyExtension.java line > 377: > >> 375: // If we are keeping state, see if the identity is >> in the cache >> 376: if (requestedId.identity.length == >> SessionId.MAX_LENGTH) { >> 377: synchronized (sessionCache) { > > Did you have a test if there is a performance regression by introducing the > synchronization here? > > I have to check the engineGetServerSessionContext() specification and > implementation (if the sessionCache is a reference?), and the session cache > implementation to make sure the synchronization works (if the synchronization > on sessionCache is the same as the synchronization on the cache internal > implementation) . Maybe, the improvement could in the cache implementation > for readability? Yes, I’ve made a test that calculates total time spent by server to receive <N> connections. Every server handshake is performed in a separate thread The client starts <T> threads. Every thread sends one initial connection and <R-1> renegotiated connections. So, the total number of connections is <N>=<T>*<R> Results for the original and JDK-8241248 code are almost identical: <T>=10 <R>=100 Original OpenJDK: 1140 ms JDK-8241248 code: 1090 ms <T>=50 <R>=100 Original OpenJDK: 1164 ms JDK-8241248 code: 1108 ms <T>=60 <R>=100 Original OpenJDK: 1461 ms JDK-8241248 code: 1579 ms <T>=70 <R>=100 Original OpenJDK: 2058 ms JDK-8241248 code: 1999 ms <T>=80 <R>=100 Original OpenJDK: 2148 ms JDK-8241248 code: 2125 ms <T>=90 <R>=100 Original OpenJDK: 2540 ms JDK-8241248 code: 2514 ms <T>=90 <R>=100 Original OpenJDK: 3011 ms JDK-8241248 code: 3010 ms I can confirm that the synchronization code works. Without it I still catches exception from different threads trying to resume the same session from the cache ------------- PR: https://git.openjdk.java.net/jdk/pull/3664
