diff --git a/src/crypto/tls/conn.go b/src/crypto/tls/conn.go index ec5c903c03..dc3b8911c7 100644 --- a/src/crypto/tls/conn.go +++ b/src/crypto/tls/conn.go @@ -28,15 +28,11 @@ type Conn struct { isClient bool // constant after handshake; protected by handshakeMutex - 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 - vers uint16 // TLS version - haveVers bool // version has been negotiated - config *Config // configuration passed to constructor + handshakeMutex sync.Mutex + handshakeErr error // error resulting from handshake + vers uint16 // TLS version + haveVers bool // version has been negotiated + config *Config // configuration passed to constructor // handshakeComplete is true if the connection is currently transferring // application data (i.e. is not currently processing a handshake). handshakeComplete bool @@ -84,7 +80,7 @@ type Conn struct { clientProtocolFallback bool // input/output - in, out halfConn // in.Mutex < out.Mutex + in, out halfConn rawInput *block // raw input, right off the wire input *block // application data waiting to be read hand bytes.Buffer // handshake data waiting to be read @@ -566,7 +562,6 @@ func (c *Conn) newRecordHeaderError(msg string) (err RecordHeaderError) { // readRecord reads the next TLS record from the connection // and updates the record layer state. -// c.in.Mutex <= L; c.input == nil. func (c *Conn) readRecord(want recordType) error { // Caller must be in sync with connection: // handshake data if handshake not yet completed, @@ -738,7 +733,6 @@ Again: } // sendAlert sends a TLS alert message. -// c.out.Mutex <= L. func (c *Conn) sendAlertLocked(err alert) error { switch err { case alertNoRenegotiation, alertCloseNotify: @@ -758,7 +752,6 @@ func (c *Conn) sendAlertLocked(err alert) error { } // sendAlert sends a TLS alert message. -// L < c.out.Mutex. func (c *Conn) sendAlert(err alert) error { c.out.Lock() defer c.out.Unlock() @@ -795,8 +788,6 @@ const ( // // In the interests of simplicity and determinism, this code does not attempt // to reset the record size once the connection is idle, however. -// -// c.out.Mutex <= L. func (c *Conn) maxPayloadSizeForWrite(typ recordType, explicitIVLen int) int { if c.config.DynamicRecordSizingDisabled || typ != recordTypeApplicationData { return maxPlaintext @@ -846,7 +837,6 @@ func (c *Conn) maxPayloadSizeForWrite(typ recordType, explicitIVLen int) int { return n } -// c.out.Mutex <= L. func (c *Conn) write(data []byte) (int, error) { if c.buffering { c.sendBuf = append(c.sendBuf, data...) @@ -872,7 +862,6 @@ func (c *Conn) flush() (int, error) { // writeRecordLocked writes a TLS record with the given type and payload to the // connection and updates the record layer state. -// c.out.Mutex <= L. func (c *Conn) writeRecordLocked(typ recordType, data []byte) (int, error) { b := c.out.newBlock() defer c.out.freeBlock(b) @@ -948,7 +937,6 @@ func (c *Conn) writeRecordLocked(typ recordType, data []byte) (int, error) { // writeRecord writes a TLS record with the given type and payload to the // connection and updates the record layer state. -// L < c.out.Mutex. func (c *Conn) writeRecord(typ recordType, data []byte) (int, error) { c.out.Lock() defer c.out.Unlock() @@ -958,7 +946,6 @@ func (c *Conn) writeRecord(typ recordType, data []byte) (int, error) { // readHandshake reads the next handshake message from // the record layer. -// c.in.Mutex < L; c.out.Mutex < L. func (c *Conn) readHandshake() (interface{}, error) { for c.hand.Len() < 4 { if err := c.in.err; err != nil { @@ -1094,7 +1081,6 @@ func (c *Conn) Write(b []byte) (int, error) { } // handleRenegotiation processes a HelloRequest handshake message. -// c.in.Mutex <= L func (c *Conn) handleRenegotiation() error { msg, err := c.readHandshake() if err != nil { @@ -1272,61 +1258,19 @@ func (c *Conn) closeNotify() error { // Most uses of this package need not call Handshake // explicitly: the first Read or Write will call it automatically. func (c *Conn) Handshake() error { - // c.handshakeErr and c.handshakeComplete 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() defer c.handshakeMutex.Unlock() - for { - if err := c.handshakeErr; err != nil { - return err - } - if c.handshakeComplete { - return nil - } - if c.handshakeCond == nil { - break - } - - c.handshakeCond.Wait() + if err := c.handshakeErr; err != nil { + return err + } + if c.handshakeComplete { + return nil } - - // 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() defer c.in.Unlock() - c.handshakeMutex.Lock() - - // The handshake cannot have completed when handshakeMutex was unlocked - // because this goroutine set handshakeCond. - if c.handshakeErr != nil || c.handshakeComplete { - panic("handshake should not have been able to complete after handshakeCond was set") - } - if c.isClient { c.handshakeErr = c.clientHandshake() } else { @@ -1344,11 +1288,6 @@ func (c *Conn) Handshake() error { 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 } diff --git a/src/crypto/tls/handshake_client.go b/src/crypto/tls/handshake_client.go index 62bb1d06c6..fd6df4251c 100644 --- a/src/crypto/tls/handshake_client.go +++ b/src/crypto/tls/handshake_client.go @@ -91,7 +91,6 @@ NextCipherSuite: return hello, nil } -// c.out.Mutex <= L; c.handshakeMutex <= L. func (c *Conn) clientHandshake() error { if c.config == nil { c.config = defaultConfig() diff --git a/src/crypto/tls/handshake_server.go b/src/crypto/tls/handshake_server.go index dfb9a59a16..f8dd630a04 100644 --- a/src/crypto/tls/handshake_server.go +++ b/src/crypto/tls/handshake_server.go @@ -36,7 +36,6 @@ type serverHandshakeState struct { } // serverHandshake performs a TLS handshake as a server. -// c.out.Mutex <= L; c.handshakeMutex <= L. func (c *Conn) serverHandshake() error { // If this is the first server handshake, we generate a random key to // encrypt the tickets with.