Code review request, 8013809 deadlock in SSLSocketImpl between between write and close

[Xuelei Fan]

If no objections, I will push the changeset, and request to back the fix.

Thanks,
Xuelei

On 8/2/2013 9:12 AM, Xuelei Fan wrote:
> On 8/2/2013 8:45 AM, Brad Wetmore wrote:
>>
>>
>> On 7/25/2013 2:11 AM, Xuelei Fan wrote:
>>> Hi Brad,
>>>
>>> Are you available to review this fix?
>>>
>>> Webrev: http://cr.openjdk.java.net/~xuelei/8013809/webrev.00/
>>>
>>> No new regression test, hard to reproduce the issue.
>>
>> Your immediate fix looks good, however, IIRC, the reason for having
>> getConnectionState() was to provide synchronized access (and syncing on
>> the SSLSocketImpl object) to the connectionState variable before the
>> Java "volatile" semantics were cleaned up in JDK 1.5.
>>
> I think there is a lot of different of a "volatile" object and a
> synchronized block.  A volatile object is to make sure the change and
> access of the object is synchronized; but synchronized block also make
> sure that the full process of the change is synchronized. For example:
> 
>       synchronized (aLock) {
>          // We want to change the object, but may need
>          // to prepare something before the change, please
>          // don't access this object while we make the preparation.
> 
>          ... // prepare something else.
> 
>          aObject = newvalue;
>       }
> 
>> Now that you've made this change, does it make sense to get rid of
>> getConnectionState(). What do you think?
>>
> I was hesitated to remove the getConnectionState().  When we change the
> value of connectionState, "this" object is locked.  If the value change
> does not completed, other thread cannot access the state.  If we remove
> this synchronization, the behavior would be changed significantly.
> 
> The reason that we can make the change in isClosed() implementation is
> that "APP_CLOSED" is the final state of the life-cycle of connection,
> therefore we don't care too much whether there is another thread is
> doing the closing or not.  It looks like a little tricky.
> 
> Xuelei
> 
>> I hate race conditions!  ;)
>>
>> Brad
>>
>>
>