RFR: 8273553: sun.security.ssl.SSLEngineImpl.closeInbound also has similar error of JDK-8253368 [v2]

Xue-Lei Andrew Fan xuelei at openjdk.java.net
Thu Mar 24 06:51:51 UTC 2022


On Wed, 23 Mar 2022 18:09:46 GMT, Xue-Lei Andrew Fan <xuelei at openjdk.org> wrote:

>> Bradford Wetmore 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 12 additional commits since the last revision:
>> 
>>  - Code review comment: enclose conContext.closeInbound() in a try/finally block.
>>  - Merge branch 'master' into JDK-8273553
>>  - Merge branch 'master' into JDK-8273553
>>  - Added SSLSocket bugid since we're actually checking both sides now.
>>  - I/O Issues, rewrite the I/O section so that early Socket closes don't kill our server-side reads.
>>  - Merge branch 'master' into JDK-8273553
>>  - Merge branch 'master' into JDK-8273553
>>  - Merge
>>  - Minor test tweaks.
>>  - Remove inadvertent whitespace
>>  - ... and 2 more: https://git.openjdk.java.net/jdk/compare/2cf735d5...b2f64d92
>
> src/java.base/share/classes/sun/security/ssl/SSLEngineImpl.java line 802:
> 
>> 800:             } finally {
>> 801:                 engineLock.unlock();
>> 802:             }
> 
> I looked the update further.  It would be nice if the try-statement could support more than one finally blocks in the future so that the code could be more clear.
> 
> try {
>     ...
> } finally {
>    // the 1st final block
> } finally {
>    // the 2nd final block
> }
> 
> 
> For the block from 781 to 787, it may be more effective if moving the block out of the synchronization lock (that's, moving 781-787 to line 779.)

> @XueleiFan, I'm not following your first comment.

Sorry for that.  I was wondering something new in the future, which is not doable in the current Java language.  Please just leave it alone.

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

PR: https://git.openjdk.java.net/jdk/pull/7796



More information about the security-dev mailing list