On Thu, 13 Jul 2023 12:11:09 GMT, Matthew Donovan <[email protected]> wrote:
>> I reverted the changes to SSLEngineImpl and looked at the history of >> SSLSocketImpl and TransportContext but didn't see anything that I would be >> undoing with this change. > > Sorry for the delay on any updates here. > > I updated this branch and verified the tests still pass. I ran jdk_security3 > tests from test/jdk/TEST.groups. Is there anything else I should do to test > this change? SSLSocketImpl uses the locks similar to SSLEngineImpl. It may get it complicated and hard to maintain if using different locks in SSLSocketImpl and SSLEngineImpl. You may be able to find similar locking scenarios in SSLSocket implementation like: SSLEngineImpl.DelegatedTask.run()->TransportContext.dispatch()->SSLHandshake().consume()->HandshakeConsumer.consume()->SSLExtensions.product()->ServerHello.produce()->shc.handshakeSession = session. I understand where you came from for the NullPointerException in the bug. But it is hardly the direction to change the overall locking scenarios. Maybe, the close() function implementation could be focused on instead. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/13742#discussion_r1262802858
