Thanks Sean, Alexey,
On 08/07/2020 13:25, Sean Mullan wrote:
Updated webrev : http://cr.openjdk.java.net/~abakhtin/8245527/webrev.v12/
You will also need to update the CSR to remove the connection timeout
property.
Also, you should document the "com.sun.jndi.ldap.tls.cbtype" property in
the java.naming module-info file as was done for other properties in
Daniel's recent RFR, once he has pushed it [1].
I have pushed the fix:
https://hg.openjdk.java.net/jdk/jdk/rev/ed375ae6c779
Alexey, you should now be able to document your new
"com.sun.jndi.ldap.tls.cbtype"
property in that @implNote as well, and update your CSR
consequently.
* src/java.naming/share/classes/com/sun/jndi/ldap/Connection.java
It looks like there is a possibility of deadlock if
getTlsServerCertificate() is called before handshakeCompleted(). In that
case the lock could be obtained first and the thread would end up
holding the lock and waiting forever.
I also have a concern with the use of wait/notify in that code.
I would suggest prototyping using a CompletableFuture instead
(or can new certificates be exchanged later on the same
connection?)
CompletableFuture would allow you to relay and propagate any
exception raised in the callback handler as well, and would
avoid running into deadlocks.
best regards,
-- daniel