Merge lp:~pedronis/ubuntu-push/close-race into lp:ubuntu-push/automatic

Proposed by Samuele Pedroni
Status: Merged
Approved by: Samuele Pedroni
Approved revision: 101
Merged at revision: 97
Proposed branch: lp:~pedronis/ubuntu-push/close-race
Merge into: lp:ubuntu-push/automatic
Prerequisite: lp:~pedronis/ubuntu-push/try-hosts
Diff against target: 78 lines (+19/-3)
2 files modified
client/session/session.go (+18/-2)
client/session/session_test.go (+1/-1)
To merge this branch: bzr merge lp:~pedronis/ubuntu-push/close-race
Reviewer Review Type Date Requested Status
John Lenton (community) Approve
Review via email: mp+213224@code.launchpad.net

Commit message

fix race when Close and Dial aren't on the same goroutine

Description of the change

fix race when Close and Dial aren't on the same goroutine

To post a comment you must log in.
Revision history for this message
John Lenton (chipaca) :
review: Approve
lp:~pedronis/ubuntu-push/close-race updated
101. By Samuele Pedroni

Merged try-hosts into close-race.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'client/session/session.go'
2--- client/session/session.go 2014-03-28 14:48:53 +0000
3+++ client/session/session.go 2014-03-28 14:48:53 +0000
4@@ -26,6 +26,7 @@
5 "math/rand"
6 "net"
7 "strings"
8+ "sync"
9 "sync/atomic"
10 "time"
11
12@@ -92,6 +93,7 @@
13 // hook for testing
14 timeSince func(time.Time) time.Duration
15 // connection
16+ connLock sync.RWMutex
17 Connection net.Conn
18 Log logger.Logger
19 TLS *tls.Config
20@@ -150,6 +152,18 @@
21 atomic.StoreUint32(sess.stateP, uint32(state))
22 }
23
24+func (sess *ClientSession) setConnection(conn net.Conn) {
25+ sess.connLock.Lock()
26+ defer sess.connLock.Unlock()
27+ sess.Connection = conn
28+}
29+
30+func (sess *ClientSession) getConnection() net.Conn {
31+ sess.connLock.RLock()
32+ defer sess.connLock.RUnlock()
33+ return sess.Connection
34+}
35+
36 // getHosts sets deliverHosts possibly querying a remote endpoint
37 func (sess *ClientSession) getHosts() error {
38 if sess.getHost != nil {
39@@ -217,7 +231,7 @@
40 break
41 }
42 }
43- sess.Connection = tls.Client(conn, sess.TLS)
44+ sess.setConnection(tls.Client(conn, sess.TLS))
45 sess.setState(Connected)
46 return nil
47 }
48@@ -240,6 +254,8 @@
49 sess.doClose()
50 }
51 func (sess *ClientSession) doClose() {
52+ sess.connLock.Lock()
53+ defer sess.connLock.Unlock()
54 if sess.Connection != nil {
55 sess.Connection.Close()
56 // we ignore Close errors, on purpose (the thinking being that
57@@ -319,7 +335,7 @@
58
59 // Call this when you've connected and want to start looping.
60 func (sess *ClientSession) start() error {
61- conn := sess.Connection
62+ conn := sess.getConnection()
63 err := conn.SetDeadline(time.Now().Add(sess.ExchangeTimeout))
64 if err != nil {
65 sess.setState(Error)
66
67=== modified file 'client/session/session_test.go'
68--- client/session/session_test.go 2014-03-28 14:48:53 +0000
69+++ client/session/session_test.go 2014-03-28 14:48:53 +0000
70@@ -1095,7 +1095,7 @@
71 c.Assert(err, IsNil)
72 sess, err := NewSession(lst.Addr().String(), nil, timeout, "wah", cs.lvls, cs.log)
73 c.Assert(err, IsNil)
74- //defer sess.Close() xxx provokes a race, fix in a later branch
75+ defer sess.Close()
76
77 upCh := make(chan interface{}, 5)
78 downCh := make(chan interface{}, 5)

Subscribers

People subscribed via source and target branches