mirror of
https://github.com/golang/go.git
synced 2025-12-08 06:10:04 +00:00
crypto/tls: fix deadlock when racing to complete handshake.
After renegotiation support was added (af125a5193) it's possible for a
Write to block on a Read when racing to complete the handshake:
1. The Write determines that a handshake is needed and tries to
take the neccesary locks in the correct order.
2. The Read also determines that a handshake is needed and wins
the race to take the locks.
3. The Read goroutine completes the handshake and wins a race
to unlock and relock c.in, which it'll hold when waiting for
more network data.
If the application-level protocol requires the Write to complete before
data can be read then the system as a whole will deadlock.
Unfortunately it doesn't appear possible to reverse the locking order of
c.in and handshakeMutex because we might read a renegotiation request at
any point and need to be able to do a handshake without unlocking.
So this change adds a sync.Cond that indicates that a goroutine has
committed to doing a handshake. Other interested goroutines can wait on
that Cond when needed.
The test for this isn't great. I was able to reproduce the deadlock with
it only when building with -race. (Because -race happened to alter the
timing just enough.)
Fixes #17101.
Change-Id: I4e8757f7b82a84e46c9963a977d089f0fb675495
Reviewed-on: https://go-review.googlesource.com/29164
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
This commit is contained in:
parent
ad5d91c17a
commit
254169d7bb
2 changed files with 107 additions and 15 deletions
|
|
@ -1168,3 +1168,57 @@ func TestAlertFlushing(t *testing.T) {
|
|||
t.Errorf("expected server handshake to complete with one write, but saw %d", n)
|
||||
}
|
||||
}
|
||||
|
||||
func TestHandshakeRace(t *testing.T) {
|
||||
// This test races a Read and Write to try and complete a handshake in
|
||||
// order to provide some evidence that there are no races or deadlocks
|
||||
// in the handshake locking.
|
||||
for i := 0; i < 32; i++ {
|
||||
c, s := net.Pipe()
|
||||
|
||||
go func() {
|
||||
server := Server(s, testConfig)
|
||||
if err := server.Handshake(); err != nil {
|
||||
panic(err)
|
||||
}
|
||||
|
||||
var request [1]byte
|
||||
if n, err := server.Read(request[:]); err != nil || n != 1 {
|
||||
panic(err)
|
||||
}
|
||||
|
||||
server.Write(request[:])
|
||||
server.Close()
|
||||
}()
|
||||
|
||||
startWrite := make(chan struct{})
|
||||
startRead := make(chan struct{})
|
||||
readDone := make(chan struct{})
|
||||
|
||||
client := Client(c, testConfig)
|
||||
go func() {
|
||||
<-startWrite
|
||||
var request [1]byte
|
||||
client.Write(request[:])
|
||||
}()
|
||||
|
||||
go func() {
|
||||
<-startRead
|
||||
var reply [1]byte
|
||||
if n, err := client.Read(reply[:]); err != nil || n != 1 {
|
||||
panic(err)
|
||||
}
|
||||
c.Close()
|
||||
readDone <- struct{}{}
|
||||
}()
|
||||
|
||||
if i&1 == 1 {
|
||||
startWrite <- struct{}{}
|
||||
startRead <- struct{}{}
|
||||
} else {
|
||||
startRead <- struct{}{}
|
||||
startWrite <- struct{}{}
|
||||
}
|
||||
<-readDone
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue