<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
</head>
<body>
<div dir="ltr">
<div></div>
<div>
<div>I agree with that statement as an author of applications which have to connect to a wide range of external systems with a wide range of libraries and components. It should be configurable as a parameter on socket, session or factory level.</div>
<div dir="ltr"><br>
</div>
<div dir="ltr">I find a new API is more than welcome, especially if it is not only synchron but also provides context and interaction (especially deny a handshake for rate limiting purposes)</div>
<div dir="ltr"><br>
</div>
<div dir="ltr">Gruß</div>
<div dir="ltr">Bernd</div>
<div><br>
</div>
<div class="ms-outlook-ios-signature" id="ms-outlook-mobile-signature">
<div style="direction: ltr">-- </div>
<div style="direction: ltr">https://Bernd.eckenfels.net</div>
</div>
</div>
</div>
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>Von:</b> security-dev <security-dev-retn@openjdk.java.net> im Auftrag von Carter Kozak <ckozak@ckozak.net><br>
<b>Gesendet:</b> Friday, September 18, 2020 8:09:09 PM<br>
<b>An:</b> Xuelei Fan <xuelei.fan@oracle.com>; Bradford Wetmore <bradford.wetmore@oracle.com>; security-dev@openjdk.java.net <security-dev@openjdk.java.net><br>
<b>Betreff:</b> Re: SSLSocket HandshakeCompletionListener Threading</font>
<div> </div>
</div>
<div class="BodyFragment"><font size="2"><span style="font-size:11pt;">
<div class="PlainText">Thanks Xuelei,<br>
<br>
As someone who primarily produces libraries consumed across many projects I would prefer not to use global jvm state. I worry that a global property requires administrators to have knowledge of every HandshakeCompletedListener that is used, and library authors
have limited ability to take advantage of the optimization. Using loom virtual threads is likely good enough, otherwise I can put together proposals for per-SSLSocket and per-HandshakeCompletionListner executor configuration if that's helpful.<br>
<br>
Thanks,<br>
Carter Kozak<br>
<br>
On Thu, Sep 17, 2020, at 15:08, Xuelei Fan wrote:<br>
> I thought more about the problem. I could see the performance <br>
> improvement requirements for heavy loaded system. But it may not worthy <br>
> of a complicated proposal, as the Loom feature could mitigate the impact.<br>
> <br>
> Maybe, a new system property could be defined, for example, <br>
> "jdk.tls.server/client.handshakeCompletedListener.useNewThread", to turn <br>
> on/off the thread creating in JDK for HandshakeCompletedEvent distribution.<br>
> <br>
> if (useNewThread) {<br>
> // use Loom improved threading<br>
> } else {<br>
> // No thread created, distributed the event to each listener, the<br>
> // listener will take care of the following execution, w/o thread.<br>
> for each listener {<br>
> listener.handshakeCompleted(event);<br>
> }<br>
> }<br>
> <br>
> There are a few drawbacks as the impact of System property is JVM <br>
> globally, but should be fine.<br>
> <br>
> Xuelei<br>
> <br>
> On 9/16/2020 8:49 PM, Bradford Wetmore wrote:<br>
> > From a coding point of view, if Xuelei doesn't have a further <br>
> > suggestion, using virtual threads like you have suggested seems to be a <br>
> > good solution. I'm ok with this change for Project Loom. Loom has a <br>
> > lot of promise for things like this.<br>
> > <br>
> > I've never been thrilled with the threading of the <br>
> > HandshakeCompletedListener and subsequent proposals, but unfortunately <br>
> > this was something we inherited long ago. That's one of the reasons I <br>
> > didn't include it in SSLEngine, but rather made a FINISHED <br>
> > HandshakeStatus Event Type.<br>
> > <br>
> > Thanks,<br>
> > <br>
> > Brad<br>
> > <br>
> > <br>
> > <br>
> > On 9/16/2020 12:21 PM, Xuelei Fan wrote:<br>
> >> Good catch!<br>
> >><br>
> >> Alternatively, I was wondering if it is possible to delegate the job <br>
> >> to listeners, without modify the APIs, for example by implementing a <br>
> >> Runnable interface (not a proposal, just a guess for now). I don't <br>
> >> like the creation of threads in the JSSE provider, as application <br>
> >> could take better care of the resources.<br>
> >><br>
> >> I need more time to think about it.<br>
> >><br>
> >> Xuelei<br>
> >><br>
> >> On 9/16/2020 7:39 AM, Carter Kozak wrote:<br>
> >>> Hello,<br>
> >>><br>
> >>> SSLSocket HandshakeCompletionListeners are a well known performance <br>
> >>> bottleneck due to new thread creation for each handshake, and the <br>
> >>> resulting session may be invalid by the time the listener thread has <br>
> >>> begun.<br>
> >>><br>
> >>> Prior discussions:<br>
> >>><br>
> >>> <a href="https://mail.openjdk.java.net/pipermail/security-dev/2020-July/022220.html">
https://mail.openjdk.java.net/pipermail/security-dev/2020-July/022220.html</a> <br>
> >>><br>
> >>> <a href="https://bugs.openjdk.java.net/browse/JDK-8246039">https://bugs.openjdk.java.net/browse/JDK-8246039</a><br>
> >>> <a href="https://github.com/openjdk/loom/pull/16">https://github.com/openjdk/loom/pull/16</a><br>
> >>><br>
> >>> Alan Bateman has suggested that we should re-validate calling <br>
> >>> listeners on separate threads because the resulting session may no <br>
> >>> longer be valid, and listeners themselves are capable of submitting <br>
> >>> work to an executor if they prefer. However I'm not confident we can <br>
> >>> safely change the implementation of the existing API without breaking <br>
> >>> consumers. It's reasonable to log handshake diagnostic information <br>
> >>> from a listener where it's not necessary for the session to be up to <br>
> >>> date, however without running asynchronously an https network logging <br>
> >>> appender may deadlock itself if the current implementation is updated <br>
> >>> to run all listeners on the same thread.<br>
> >>><br>
> >>> Another option is to provide an overload of <br>
> >>> SSLSocket.addHandshakeCompletedListener which takes both a <br>
> >>> HandshakeCompletedListener and an Executor. An executor may be chosen <br>
> >>> to run listeners on the calling thread (executor Runnable::run), or <br>
> >>> an executor capable of pooling threads. There's some risk that this <br>
> >>> API could be used improperly and create a deadlock as described in <br>
> >>> the logging example, but with great power comes great responsibility <br>
> >>> and the upsides seem to outweigh the potential risk, especially given <br>
> >>> the thread-explosion problems we're currently experiencing.<br>
> >>><br>
> >>> In the Loom PR linked above I've begun by attempting to preserve the <br>
> >>> existing behavior while reducing the cost of a listener when loom is <br>
> >>> available, using virtual threads instead of OS threads. Any and all <br>
> >>> feedback is greatly appreciated.<br>
> >>><br>
> >>> Thanks,<br>
> >>> Carter Kozak<br>
> >>><br>
> <br>
</div>
</span></font></div>
</body>
</html>