On Fri, 14 Jul 2023 20:00:55 GMT, Matthew Donovan 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
>> synchronize
> 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
Matthew Donovan has updated the pull re
On Fri, 14 Jul 2023 16:44:10 GMT, Xue-Lei Andrew Fan wrote:
>> The only lock I added was in `TransportContext` to synchronize access to the
>> `handshakeContext` field, but I understand your reluctance to make any
>> changes with locks. The problem is that SSLSocketImpl tries to access
>> `con
On Fri, 14 Jul 2023 14:16:00 GMT, Matthew Donovan wrote:
> TransportContext also has a `protocolVersion` field. Is it possible to just
> use that instead?
Did you mean a change in duplexCloseOutput() like the following?
- // The protocol version may have been negotiated.
-
On Thu, 13 Jul 2023 16:31:41 GMT, Xue-Lei Andrew Fan wrote:
>> 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?
>
> SSLSock
On Thu, 13 Jul 2023 12:11:09 GMT, Matthew Donovan 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 update
On Wed, 21 Jun 2023 12:04:34 GMT, Matthew Donovan wrote:
>> There's a bit of a history with SSLSocket closures since the new handshaker
>> was brought into JDK11. Some of it dealt with synchronization, others with
>> properly handling full vs. half-duplex closes. You may want to look up some
> 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
Matthew Donovan has updated the pull re
On Fri, 16 Jun 2023 14:58:06 GMT, Jamil Nimeh wrote:
>> Yes, socket close is a headache problem for me.
>
> There's a bit of a history with SSLSocket closures since the new handshaker
> was brought into JDK11. Some of it dealt with synchronization, others with
> properly handling full vs. half
On Fri, 16 Jun 2023 14:52:44 GMT, Xue-Lei Andrew Fan wrote:
>> There is a difference, though. The close() method in SSLSocketImpl is not
>> synchronized and there is the chance of a NullPointerException in
>> duplexCloseOutput() because `conContext.handshakeContext` is being set to
>> null by
On Fri, 16 Jun 2023 14:45:52 GMT, Matthew Donovan wrote:
>> I may update both SSLSocketImpl and SSLEngineImpl, as SSLSocketImpl uses the
>> locks similar to SSLEngineImpl.
>
> There is a difference, though. The close() method in SSLSocketImpl is not
> synchronized and there is the chance of a N
On Fri, 16 Jun 2023 14:30:33 GMT, Xue-Lei Andrew Fan wrote:
>> I see what you're saying with respect to SSLEngineImpl. It looks like all of
>> the public methods are synchronized with `engineLock` so I shouldn't have
>> made any changes there. I will revert them.
>>
>> Are you ok with the chan
On Fri, 16 Jun 2023 14:24:17 GMT, Matthew Donovan wrote:
>> I had a search of the value assignment of handshakeSession. Here is one
>> example of the calling stack:
>> SSLEngineImpl.DelegatedTask.run()->TransportContext.dispatch()->SSLHandshake().consume()->HandshakeConsumer.consume()->SSLExten
On Fri, 16 Jun 2023 13:54:37 GMT, Xue-Lei Andrew Fan wrote:
>> `engineLock` was used to here to make sure that
>> `conContext.handshakeContext` is not null before accessing it. The problem
>> is that the `handshakeContext` is modified (set to null) by the
>> `TransportContext` class. So using
On Fri, 16 Jun 2023 11:59:01 GMT, Matthew Donovan wrote:
>> src/java.base/share/classes/sun/security/ssl/SSLEngineImpl.java line 914:
>>
>>> 912: @Override
>>> 913: public SSLSession getHandshakeSession() {
>>> 914: engineLock.lock();
>>
>> I'm not very sure of this update. By r
On Fri, 16 Jun 2023 06:24:36 GMT, Xue-Lei Andrew Fan wrote:
>> Matthew Donovan has updated the pull request with a new target base due to a
>> merge or a rebase. The incremental webrev excludes the unrelated changes
>> brought in by the merge/rebase. The pull request contains five additional
>
> 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
Matthew Donovan has updated the pull re
On Wed, 3 May 2023 11:26:32 GMT, Matthew Donovan 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
On Wed, 3 May 2023 11:26:32 GMT, Matthew Donovan 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
On Wed, 3 May 2023 11:26:32 GMT, Matthew Donovan 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
On Wed, 3 May 2023 11:26:32 GMT, Matthew Donovan 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
On Wed, 3 May 2023 11:26:32 GMT, Matthew Donovan 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
> 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
Matthew Donovan has updated the pull re
On Tue, 2 May 2023 14:31:38 GMT, Matthew Donovan 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
> 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
Matthew Donovan has updated the pull re
On Tue, 2 May 2023 12:56:38 GMT, Daniel Fuchs wrote:
>> Matthew Donovan has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> using try/finally in terminateHandshakeContext and using local context
>> variable in all places it should be
>
> sr
On Mon, 1 May 2023 17:39:02 GMT, Matthew Donovan 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 met
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
-
Commit messages:
- 8290005: com/sun/
28 matches
Mail list logo
