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

[Xuelei Fan]

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
> 
>