Merge lp:~jtv/gwacl/global-libcurl-lock into lp:gwacl

Proposed by Jeroen T. Vermeulen on 2013-07-17
Status: Rejected
Rejected by: Jeroen T. Vermeulen on 2013-07-17
Proposed branch: lp:~jtv/gwacl/global-libcurl-lock
Merge into: lp:gwacl
Diff against target: 31 lines (+13/-0)
1 file modified
x509dispatcher.go (+13/-0)
To merge this branch: bzr merge lp:~jtv/gwacl/global-libcurl-lock
Reviewer Review Type Date Requested Status
Jeroen T. Vermeulen (community) Resubmit on 2013-07-17
Review via email: mp+175229@code.launchpad.net

Commit message

Fix the test failures (but not the horrible crashes) that happen when upgrading from go1.0.2 to go1.1.1.

Description of the change

There are also a bunch of segfaults that Julian is currently looking into. But the actual "failures" were caused by XML serialization no longer including a leading newline.

This fix is tedious and repetitive (really, it should quite literally be the exact same fix for all cases) but it has the advantage that it works on both Go versions, and will continue to work with other similar whitespace changes.

Jeroen

To post a comment you must log in.
Jeroen T. Vermeulen (jtv) wrote :

Sorry. Proposed wrong branch here.

review: Resubmit

Unmerged revisions

188. By Jeroen T. Vermeulen on 2013-07-17

Merge trunk.

187. By Jeroen T. Vermeulen on 2013-07-17

Lock-protect all use of libcurl, because it may not be reentrant.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'x509dispatcher.go'
2--- x509dispatcher.go 2013-07-12 14:42:08 +0000
3+++ x509dispatcher.go 2013-07-17 09:17:20 +0000
4@@ -8,8 +8,14 @@
5 . "launchpad.net/gwacl/logging"
6 "net/http"
7 "net/textproto"
8+ "sync"
9 )
10
11+// globalCurlLock makes sure we don't call libcurl concurrently. We're not
12+// sure that it is safe to do so, and we've been having a few mysterious
13+// crashes.
14+var globalCurlLock sync.Mutex
15+
16 type X509Request struct {
17 APIVersion string
18 Method string
19@@ -147,6 +153,13 @@
20 Debugf("Request body: %s\n", request.Payload)
21 }
22 response := newX509Response()
23+
24+ // Protect against concurrent use of libcurl. We're not sure it's
25+ // fully reentrant, and it has been implicated in at least one of our
26+ // mysterious crashes during Juju testing.
27+ globalCurlLock.Lock()
28+ defer globalCurlLock.Unlock()
29+
30 ch := request.makeCurlRequest(session, response)
31 defer ch.Cleanup()
32

Subscribers

People subscribed via source and target branches