author | Shawn Walker-Salas <shawn.walker@oracle.com> |
Tue, 20 Dec 2016 11:59:29 -0800 | |
changeset 7518 | c388d4e1d3ad |
permissions | -rw-r--r-- |
7518
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
1 |
From ca0b97e80a558d06274e68ece667b821901acc42 Mon Sep 17 00:00:00 2001 |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
2 |
From: Adam Langley <[email protected]> |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
3 |
Date: Wed, 14 Sep 2016 11:50:36 -0700 |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
4 |
Subject: [PATCH 21/38] [release-branch.go1.7] crypto/tls: fix deadlock when |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
5 |
racing to complete handshake. |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
6 |
|
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
7 |
After renegotiation support was added (af125a5193c) it's possible for a |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
8 |
Write to block on a Read when racing to complete the handshake: |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
9 |
1. The Write determines that a handshake is needed and tries to |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
10 |
take the neccesary locks in the correct order. |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
11 |
2. The Read also determines that a handshake is needed and wins |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
12 |
the race to take the locks. |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
13 |
3. The Read goroutine completes the handshake and wins a race |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
14 |
to unlock and relock c.in, which it'll hold when waiting for |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
15 |
more network data. |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
16 |
|
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
17 |
If the application-level protocol requires the Write to complete before |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
18 |
data can be read then the system as a whole will deadlock. |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
19 |
|
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
20 |
Unfortunately it doesn't appear possible to reverse the locking order of |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
21 |
c.in and handshakeMutex because we might read a renegotiation request at |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
22 |
any point and need to be able to do a handshake without unlocking. |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
23 |
|
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
24 |
So this change adds a sync.Cond that indicates that a goroutine has |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
25 |
committed to doing a handshake. Other interested goroutines can wait on |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
26 |
that Cond when needed. |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
27 |
|
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
28 |
The test for this isn't great. I was able to reproduce the deadlock with |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
29 |
it only when building with -race. (Because -race happened to alter the |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
30 |
timing just enough.) |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
31 |
|
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
32 |
Fixes #17101. |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
33 |
|
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
34 |
Change-Id: I4e8757f7b82a84e46c9963a977d089f0fb675495 |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
35 |
Reviewed-on: https://go-review.googlesource.com/29164 |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
36 |
Reviewed-by: Brad Fitzpatrick <[email protected]> |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
37 |
Reviewed-by: Ian Lance Taylor <[email protected]> |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
38 |
Reviewed-on: https://go-review.googlesource.com/31268 |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
39 |
--- |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
40 |
src/crypto/tls/conn.go | 68 +++++++++++++++++++++++++-------- |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
41 |
src/crypto/tls/handshake_client_test.go | 54 ++++++++++++++++++++++++++ |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
42 |
2 files changed, 107 insertions(+), 15 deletions(-) |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
43 |
|
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
44 |
diff --git a/src/crypto/tls/conn.go b/src/crypto/tls/conn.go |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
45 |
index 87bef23..77fd6d3 100644 |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
46 |
--- a/src/crypto/tls/conn.go |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
47 |
+++ b/src/crypto/tls/conn.go |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
48 |
@@ -29,10 +29,14 @@ type Conn struct { |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
49 |
|
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
50 |
// constant after handshake; protected by handshakeMutex |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
51 |
handshakeMutex sync.Mutex // handshakeMutex < in.Mutex, out.Mutex, errMutex |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
52 |
- handshakeErr error // error resulting from handshake |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
53 |
- vers uint16 // TLS version |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
54 |
- haveVers bool // version has been negotiated |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
55 |
- config *Config // configuration passed to constructor |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
56 |
+ // handshakeCond, if not nil, indicates that a goroutine is committed |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
57 |
+ // to running the handshake for this Conn. Other goroutines that need |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
58 |
+ // to wait for the handshake can wait on this, under handshakeMutex. |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
59 |
+ handshakeCond *sync.Cond |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
60 |
+ handshakeErr error // error resulting from handshake |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
61 |
+ vers uint16 // TLS version |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
62 |
+ haveVers bool // version has been negotiated |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
63 |
+ config *Config // configuration passed to constructor |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
64 |
// handshakeComplete is true if the connection is currently transfering |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
65 |
// application data (i.e. is not currently processing a handshake). |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
66 |
handshakeComplete bool |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
67 |
@@ -1206,26 +1210,50 @@ func (c *Conn) Handshake() error { |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
68 |
// need to check whether a handshake is pending (such as Write) to |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
69 |
// block. |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
70 |
// |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
71 |
- // Thus we take c.handshakeMutex first and, if we find that a handshake |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
72 |
- // is needed, then we unlock, acquire c.in and c.handshakeMutex in the |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
73 |
- // correct order, and check again. |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
74 |
+ // Thus we first take c.handshakeMutex to check whether a handshake is |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
75 |
+ // needed. |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
76 |
+ // |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
77 |
+ // If so then, previously, this code would unlock handshakeMutex and |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
78 |
+ // then lock c.in and handshakeMutex in the correct order to run the |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
79 |
+ // handshake. The problem was that it was possible for a Read to |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
80 |
+ // complete the handshake once handshakeMutex was unlocked and then |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
81 |
+ // keep c.in while waiting for network data. Thus a concurrent |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
82 |
+ // operation could be blocked on c.in. |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
83 |
+ // |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
84 |
+ // Thus handshakeCond is used to signal that a goroutine is committed |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
85 |
+ // to running the handshake and other goroutines can wait on it if they |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
86 |
+ // need. handshakeCond is protected by handshakeMutex. |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
87 |
c.handshakeMutex.Lock() |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
88 |
defer c.handshakeMutex.Unlock() |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
89 |
|
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
90 |
- for i := 0; i < 2; i++ { |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
91 |
- if i == 1 { |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
92 |
- c.handshakeMutex.Unlock() |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
93 |
- c.in.Lock() |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
94 |
- defer c.in.Unlock() |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
95 |
- c.handshakeMutex.Lock() |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
96 |
- } |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
97 |
- |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
98 |
+ for { |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
99 |
if err := c.handshakeErr; err != nil { |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
100 |
return err |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
101 |
} |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
102 |
if c.handshakeComplete { |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
103 |
return nil |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
104 |
} |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
105 |
+ if c.handshakeCond == nil { |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
106 |
+ break |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
107 |
+ } |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
108 |
+ |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
109 |
+ c.handshakeCond.Wait() |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
110 |
+ } |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
111 |
+ |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
112 |
+ // Set handshakeCond to indicate that this goroutine is committing to |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
113 |
+ // running the handshake. |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
114 |
+ c.handshakeCond = sync.NewCond(&c.handshakeMutex) |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
115 |
+ c.handshakeMutex.Unlock() |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
116 |
+ |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
117 |
+ c.in.Lock() |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
118 |
+ defer c.in.Unlock() |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
119 |
+ |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
120 |
+ c.handshakeMutex.Lock() |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
121 |
+ |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
122 |
+ // The handshake cannot have completed when handshakeMutex was unlocked |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
123 |
+ // because this goroutine set handshakeCond. |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
124 |
+ if c.handshakeErr != nil || c.handshakeComplete { |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
125 |
+ panic("handshake should not have been able to complete after handshakeCond was set") |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
126 |
} |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
127 |
|
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
128 |
if c.isClient { |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
129 |
@@ -1236,6 +1264,16 @@ func (c *Conn) Handshake() error { |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
130 |
if c.handshakeErr == nil { |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
131 |
c.handshakes++ |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
132 |
} |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
133 |
+ |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
134 |
+ if c.handshakeErr == nil && !c.handshakeComplete { |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
135 |
+ panic("handshake should have had a result.") |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
136 |
+ } |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
137 |
+ |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
138 |
+ // Wake any other goroutines that are waiting for this handshake to |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
139 |
+ // complete. |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
140 |
+ c.handshakeCond.Broadcast() |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
141 |
+ c.handshakeCond = nil |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
142 |
+ |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
143 |
return c.handshakeErr |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
144 |
} |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
145 |
|
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
146 |
diff --git a/src/crypto/tls/handshake_client_test.go b/src/crypto/tls/handshake_client_test.go |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
147 |
index ce987f1..07d31b6 100644 |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
148 |
--- a/src/crypto/tls/handshake_client_test.go |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
149 |
+++ b/src/crypto/tls/handshake_client_test.go |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
150 |
@@ -1045,3 +1045,57 @@ func TestBuffering(t *testing.T) { |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
151 |
t.Errorf("expected server handshake to complete with only two writes, but saw %d", n) |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
152 |
} |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
153 |
} |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
154 |
+ |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
155 |
+func TestHandshakeRace(t *testing.T) { |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
156 |
+ // This test races a Read and Write to try and complete a handshake in |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
157 |
+ // order to provide some evidence that there are no races or deadlocks |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
158 |
+ // in the handshake locking. |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
159 |
+ for i := 0; i < 32; i++ { |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
160 |
+ c, s := net.Pipe() |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
161 |
+ |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
162 |
+ go func() { |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
163 |
+ server := Server(s, testConfig) |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
164 |
+ if err := server.Handshake(); err != nil { |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
165 |
+ panic(err) |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
166 |
+ } |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
167 |
+ |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
168 |
+ var request [1]byte |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
169 |
+ if n, err := server.Read(request[:]); err != nil || n != 1 { |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
170 |
+ panic(err) |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
171 |
+ } |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
172 |
+ |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
173 |
+ server.Write(request[:]) |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
174 |
+ server.Close() |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
175 |
+ }() |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
176 |
+ |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
177 |
+ startWrite := make(chan struct{}) |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
178 |
+ startRead := make(chan struct{}) |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
179 |
+ readDone := make(chan struct{}) |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
180 |
+ |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
181 |
+ client := Client(c, testConfig) |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
182 |
+ go func() { |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
183 |
+ <-startWrite |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
184 |
+ var request [1]byte |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
185 |
+ client.Write(request[:]) |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
186 |
+ }() |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
187 |
+ |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
188 |
+ go func() { |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
189 |
+ <-startRead |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
190 |
+ var reply [1]byte |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
191 |
+ if n, err := client.Read(reply[:]); err != nil || n != 1 { |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
192 |
+ panic(err) |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
193 |
+ } |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
194 |
+ c.Close() |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
195 |
+ readDone <- struct{}{} |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
196 |
+ }() |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
197 |
+ |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
198 |
+ if i&1 == 1 { |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
199 |
+ startWrite <- struct{}{} |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
200 |
+ startRead <- struct{}{} |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
201 |
+ } else { |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
202 |
+ startRead <- struct{}{} |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
203 |
+ startWrite <- struct{}{} |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
204 |
+ } |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
205 |
+ <-readDone |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
206 |
+ } |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
207 |
+} |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
208 |
-- |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
209 |
2.7.4 |
c388d4e1d3ad
PSARC/2016/250 Google Go version 1.7
Shawn Walker-Salas <shawn.walker@oracle.com>
parents:
diff
changeset
|
210 |