SSLSocket HandshakeCompletionListener Threading

Bradford Wetmore bradford.wetmore at oracle.com
Thu Sep 17 03:49:46 UTC 2020


 From a coding point of view, if Xuelei doesn't have a further 
suggestion, using virtual threads like you have suggested seems to be a 
good solution.  I'm ok with this change for Project Loom.  Loom has a 
lot of promise for things like this.

I've never been thrilled with the threading of the 
HandshakeCompletedListener and subsequent proposals, but unfortunately 
this was something we inherited long ago.  That's one of the reasons I 
didn't include it in SSLEngine, but rather made a FINISHED 
HandshakeStatus Event Type.

Thanks,

Brad



On 9/16/2020 12:21 PM, Xuelei Fan wrote:
> Good catch!
> 
> Alternatively, I was wondering if it is possible to delegate the job to 
> listeners, without modify the APIs, for example by implementing a 
> Runnable interface (not a proposal, just a guess for now).  I don't like 
> the creation of threads in the JSSE provider, as application could take 
> better care of the resources.
> 
> I need more time to think about it.
> 
> Xuelei
> 
> On 9/16/2020 7:39 AM, Carter Kozak wrote:
>> Hello,
>>
>> SSLSocket HandshakeCompletionListeners are a well known performance 
>> bottleneck due to new thread creation for each handshake, and the 
>> resulting session may be invalid by the time the listener thread has 
>> begun.
>>
>> Prior discussions:
>>
>> https://mail.openjdk.java.net/pipermail/security-dev/2020-July/022220.html 
>>
>> https://bugs.openjdk.java.net/browse/JDK-8246039
>> https://github.com/openjdk/loom/pull/16
>>
>> Alan Bateman has suggested that we should re-validate calling 
>> listeners on separate threads because the resulting session may no 
>> longer be valid, and listeners themselves are capable of submitting 
>> work to an executor if they prefer. However I'm not confident we can 
>> safely change the implementation of the existing API without breaking 
>> consumers. It's reasonable to log handshake diagnostic information 
>> from a listener where it's not necessary for the session to be up to 
>> date, however without running asynchronously an https network logging 
>> appender may deadlock itself if the current implementation is updated 
>> to run all listeners on the same thread.
>>
>> Another option is to provide an overload of 
>> SSLSocket.addHandshakeCompletedListener which takes both a 
>> HandshakeCompletedListener and an Executor. An executor may be chosen 
>> to run listeners on the calling thread (executor Runnable::run), or an 
>> executor capable of pooling threads. There's some risk that this API 
>> could be used improperly and create a deadlock as described in the 
>> logging example, but with great power comes great responsibility and 
>> the upsides seem to outweigh the potential risk, especially given the 
>> thread-explosion problems we're currently experiencing.
>>
>> In the Loom PR linked above I've begun by attempting to preserve the 
>> existing behavior while reducing the cost of a listener when loom is 
>> available, using virtual threads instead of OS threads. Any and all 
>> feedback is greatly appreciated.
>>
>> Thanks,
>> Carter Kozak
>>



More information about the security-dev mailing list