RFR: 8241248: NullPointerException in sun.security.ssl.HKDF.extract(HKDF.java:93) [v2]

Xue-Lei Andrew Fan xuelei at openjdk.java.net
Wed May 5 03:43:52 UTC 2021


On Thu, 29 Apr 2021 15:58:28 GMT, Xue-Lei Andrew Fan <xuelei at openjdk.org> wrote:

>> 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 catch exception from different threads trying to resume the same session from the cache
>
> Thank you for the update.
> 
> I also expect the code easy to read and maintain in the future.  But please go ahead for the integration if you don't want to make the update now.  We could file an enhancement later on.

> @XueleiFan , @jnimeh, thank you for your suggestions. I have updated synchronization using ReentrantLock in the Cache object. The performance test showed no issues.

Thank you for considering use ReentrantLock, the "synchronized" keyword is something we are trying to avoid in the JSSE implementation.  However, the reentrant lock introduced is not the lock in the Cache implementation.  They are overlapped and could be conflict.  The performance test looks good with just a few threads, but I'm not very sure of the through put impact if the threads goes up to 1M or more (there were complaints previously for large volume traffics).

Maybe, you could keep it simple by adding a pull() method in the Cache class (find the session and remove it from the cache if found), by using the Cache class internal synchronize methods.

-------------

PR: https://git.openjdk.java.net/jdk/pull/3664


More information about the security-dev mailing list