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