kris/th5
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

crypto/tls: simplify the Handshake locking

Browse Source 
See https://groups.google.com/forum/#!topic/golang-dev/Xxiai-R_jH0

Change-Id: I6052695ece9aff9e3112c2fb176596fde8aa9cb2
This commit is contained in:
Filippo Valsorda 2016-12-02 19:43:45 +00:00 committed by Peter Wu
parent 341de96a61
commit 1b03258899
1 changed files with 11 additions and 56 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

45
conn.go
View File

@ -33,10 +33,6 @@ type Conn struct {
// constant after handshake; protected by handshakeMutex // constant after handshake; protected by handshakeMutex
handshakeMutex sync.Mutex // handshakeMutex < in.Mutex, out.Mutex, errMutex handshakeMutex sync.Mutex // handshakeMutex < in.Mutex, out.Mutex, errMutex
// handshakeCond, if not nil, indicates that a goroutine is committed
// to running the handshake for this Conn. Other goroutines that need
// to wait for the handshake can wait on this, under handshakeMutex.
handshakeCond *sync.Cond
handshakeErr error // error resulting from handshake handshakeErr error // error resulting from handshake
connID []byte // Random connection id connID []byte // Random connection id
clientHello []byte // ClientHello packet contents clientHello []byte // ClientHello packet contents
@ -1474,55 +1470,19 @@ func (c *Conn) closeNotify() error {
// In TLS 1.3 Handshake returns after the client and server first flights, // In TLS 1.3 Handshake returns after the client and server first flights,
// without waiting for the Client Finished. // without waiting for the Client Finished.
func (c *Conn) Handshake() error { func (c *Conn) Handshake() error {
// c.handshakeErr and c.phase == handshakeRunning are protected by
// c.handshakeMutex. In order to perform a handshake, we need to lock
// c.in also and c.handshakeMutex must be locked after c.in.
//
// However, if a Read() operation is hanging then it'll be holding the
// lock on c.in and so taking it here would cause all operations that
// need to check whether a handshake is pending (such as Write) to
// block.
//
// Thus we first take c.handshakeMutex to check whether a handshake is
// needed.
//
// If so then, previously, this code would unlock handshakeMutex and
// then lock c.in and handshakeMutex in the correct order to run the
// handshake. The problem was that it was possible for a Read to
// complete the handshake once handshakeMutex was unlocked and then
// keep c.in while waiting for network data. Thus a concurrent
// operation could be blocked on c.in.
//
// Thus handshakeCond is used to signal that a goroutine is committed
// to running the handshake and other goroutines can wait on it if they
// need. handshakeCond is protected by handshakeMutex.
c.handshakeMutex.Lock() c.handshakeMutex.Lock()
defer c.handshakeMutex.Unlock() defer c.handshakeMutex.Unlock()
for {
if err := c.handshakeErr; err != nil { if err := c.handshakeErr; err != nil {
return err return err
} }
if c.handshakeComplete { if c.handshakeComplete {
return nil return nil
} }
if c.handshakeCond == nil {
break
}
c.handshakeCond.Wait()
}
// Set handshakeCond to indicate that this goroutine is committing to
// running the handshake.
c.handshakeCond = sync.NewCond(&c.handshakeMutex)
c.handshakeMutex.Unlock()
c.in.Lock() c.in.Lock()
defer c.in.Unlock() defer c.in.Unlock()
c.handshakeMutex.Lock()
// The handshake cannot have completed when handshakeMutex was unlocked // The handshake cannot have completed when handshakeMutex was unlocked
// because this goroutine set handshakeCond. // because this goroutine set handshakeCond.
if c.handshakeErr != nil || c.handshakeComplete { if c.handshakeErr != nil || c.handshakeComplete {
@ -1551,11 +1511,6 @@ func (c *Conn) Handshake() error {
panic("handshake should have had a result.") panic("handshake should have had a result.")
} }
// Wake any other goroutines that are waiting for this handshake to
// complete.
c.handshakeCond.Broadcast()
c.handshakeCond = nil
return c.handshakeErr return c.handshakeErr
} }