RFR: 8290005: com/sun/jndi/ldap/LdapCBPropertiesTest.java failling with NullPointerException

[Daniel Fuchs]

On Mon, 1 May 2023 17:39:02 GMT, Matthew Donovan <duke at openjdk.org> wrote:

> In this PR, I added methods to the TransportContext class to synchronize access to the handshakeContext field. I also updated locations in the code that rely on the handshakeContext field to not be null to use the synchronized methods.
> 
> Thanks

src/java.base/share/classes/sun/security/ssl/SSLEngineImpl.java line 767:

> 765:             if (context != null && // PRE or POST handshake
> 766:                     !conContext.handshakeContext.taskDelegated &&
> 767:                     !conContext.handshakeContext.delegatedActions.isEmpty()) {

Shouldn't you replace `conContext.handshakeContext` with `context` in those two lines as well?

src/java.base/share/classes/sun/security/ssl/TransportContext.java line 461:

> 459:         handshakeCtxLock.lock();
> 460:         handshakeContext = null;
> 461:         handshakeCtxLock.unlock();

The usual pattern is to use try { } finally { } with locks (even though in this particular case I don't see how the assignment would throw) - but if more things need to be done in the future here then at least the try { } finally { } will be in place.

Suggestion:

        handshakeCtxLock.lock();
        try {
            handshakeContext = null;
        } finally {
            handshakeCtxLock.unlock();
        }

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/13742#discussion_r1182510718
PR Review Comment: https://git.openjdk.org/jdk/pull/13742#discussion_r1182510815