SSLSocketImpl and Handshake Notifier efficiency/thread pooling

[Bernd Eckenfels]

Hello,

(Line numbers are relative to:  
http://hg.openjdk.java.net/jdk8/jdk8/jdk/file/733885f57e14/src/share/classes/sun/security/ssl/SSLSocketImpl.java  
)

while investigating the DOS resilience of SSLSocket with the Sun JSSE  
Implementation (see other mail) I found out, that the SSLSocketImpl  
implementation uses a pretty simple method to dispatch SSL handshake  
events:

- every time a handshake happens and one or more listeners are registered  
there will be a HandshakeCompletedEvent object (and a few more data  
structures cloned) created (I guess thats unavodable)

- there will be a new Thread(!) (subclass of NotifyHandshakeThread)  
created and started (Line 1081)

- the Map of Listeners will be cloned, which results in a new HashSet and  
all of its supporting classes, even when it will not be used as HashMap  
and only be used to operate over the Entries. Using a Listener[] and  
AccessContext[] array might be more efficient.


Having a Handshake Listener is most often used for counter measurements or  
statistics related to overload situations. So it is quite counter  
productive to start a new Thread (and finalize it at the end) on every  
handshake with unlimited parallelism. This with also limit the scalability  
of SocketServers as it consumes additional handles/threads on each initial  
handshake (aka connect).

The starting of the Thread is decoupling the Listener from the protocol  
engine in terms of waiting and security context. But I think in most cases  
this is not really needed as the Listener is doing very lightweight work  
(for example a .notify() or a incrementCounter(), which all would be less  
intrusive than starting a native thread. It is also not possible to name  
the Thread or control its priority or concurrency. They are started  
directly under the will of a external client (SSL renegotiation).


So I have a few suggestions/questions:



a) allow a Executor to be set on the context or socket factory (with  
default implementation using a cached thread pool executor). This would  
even allow to use the lightweight DirectExecutir if listeners are known to  
be well behaved and nonblocking

b) instead of copying the Entry set of the Listener/Context Tuples use a  
Array (less objects and faster iteration speed) (Line 2574 and Line 2581)

c) it might be possible to add the Runnable Interface to the created  
HandshakeEvent, this would reduce even more object allocations (probably  
not worth the change)

If it is not possible to configure/register the executor (because of the  
Security Manager or complexity) at least using by default a cached  
ThreadExecutor will reduce system load and increase robustness in overload  
situations.

Greetings
Bernd


BTW: I also noticed that the SSLSocketImpl is full of System.out for the  
Security Debugging (categroy "ssl") and not using the Debug.println()  
Method.
-- 
http://bernd.eckenfels.net