Merge lp:~dave-cheney/goose/tip into lp:goose

Proposed by Dave Cheney on 2014-07-02
Status: Merged
Approved by: Dave Cheney on 2014-07-02
Approved revision: 128
Merged at revision: 127
Proposed branch: lp:~dave-cheney/goose/tip
Merge into: lp:goose
Diff against target: 89 lines (+17/-0)
1 file modified
testservices/swiftservice/service.go (+17/-0)
To merge this branch: bzr merge lp:~dave-cheney/goose/tip
Reviewer Review Type Date Requested Status
Juju Engineering 2014-07-02 Pending
Review via email: mp+225257@code.launchpad.net

Commit message

testing: fix data race in swift mock

https://codereview.appspot.com/108380043/

Description of the change

testing: fix data race in swift mock

https://codereview.appspot.com/108380043/

To post a comment you must log in.
Dave Cheney (dave-cheney) wrote :

Reviewers: mp+225257_code.launchpad.net,

Message:
Please take a look.

Description:
testing: fix data race in swift mock

https://code.launchpad.net/~dave-cheney/goose/tip/+merge/225257

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/108380043/

Affected files (+19, -0 lines):
   A [revision details]
   M testservices/swiftservice/service.go

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: tarmac-20140613065325-tlt6r3jg7saby4ub
+New revision: <email address hidden>

Index: testservices/swiftservice/service.go
=== modified file 'testservices/swiftservice/service.go'
--- testservices/swiftservice/service.go 2013-09-04 09:50:54 +0000
+++ testservices/swiftservice/service.go 2014-07-02 05:34:48 +0000
@@ -10,6 +10,7 @@
   "net/url"
   "sort"
   "strings"
+ "sync"
   "time"
  )

@@ -20,6 +21,8 @@

  type Swift struct {
   testservices.ServiceInstance
+
+ mu sync.Mutex // protects the remaining fields
   containers map[string]object
  }

@@ -70,7 +73,9 @@

  // HasContainer verifies the given container exists or not.
  func (s *Swift) HasContainer(name string) bool {
+ s.mu.Lock()
   _, ok := s.containers[name]
+ s.mu.Unlock()
   return ok
  }

@@ -80,7 +85,9 @@
   if err := s.ProcessFunctionHook(s, container, name); err != nil {
    return nil, err
   }
+ s.mu.Lock()
   data, ok := s.containers[container][name]
+ s.mu.Unlock()
   if !ok {
    return nil, fmt.Errorf("no such object %q in container %q", name,
container)
   }
@@ -96,7 +103,9 @@
   if s.HasContainer(name) {
    return fmt.Errorf("container already exists %q", name)
   }
+ s.mu.Lock()
   s.containers[name] = make(object)
+ s.mu.Unlock()
   return nil
  }

@@ -110,7 +119,9 @@
   if ok := s.HasContainer(name); !ok {
    return nil, fmt.Errorf("no such container %q", name)
   }
+ s.mu.Lock()
   items := s.containers[name]
+ s.mu.Unlock()
   sorted := make([]string, 0, len(items))
   prefix := params["prefix"]
   for filename := range items {
@@ -154,7 +165,9 @@
     return err
    }
   }
+ s.mu.Lock()
   s.containers[container][name] = data
+ s.mu.Unlock()
   return nil
  }

@@ -166,7 +179,9 @@
   if ok := s.HasContainer(name); !ok {
    return fmt.Errorf("no such container %q", name)
   }
+ s.mu.Lock()
   delete(s.containers, name)
+ s.mu.Unlock()
   return nil
  }

@@ -178,7 +193,9 @@
   if _, err := s.GetObject(container, name); err != nil {
    return err
   }
+ s.mu.Lock()
   delete(s.containers[container], name)
+ s.mu.Unlock()
   return nil
  }

Andrew Wilkins (axwalk) wrote :

On 2014/07/02 05:36:39, dfc wrote:
> Please take a look.

LGTM

https://codereview.appspot.com/108380043/

Dave Cheney (dave-cheney) :

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'testservices/swiftservice/service.go'
2--- testservices/swiftservice/service.go 2013-09-04 09:50:54 +0000
3+++ testservices/swiftservice/service.go 2014-07-02 05:35:39 +0000
4@@ -10,6 +10,7 @@
5 "net/url"
6 "sort"
7 "strings"
8+ "sync"
9 "time"
10 )
11
12@@ -20,6 +21,8 @@
13
14 type Swift struct {
15 testservices.ServiceInstance
16+
17+ mu sync.Mutex // protects the remaining fields
18 containers map[string]object
19 }
20
21@@ -70,7 +73,9 @@
22
23 // HasContainer verifies the given container exists or not.
24 func (s *Swift) HasContainer(name string) bool {
25+ s.mu.Lock()
26 _, ok := s.containers[name]
27+ s.mu.Unlock()
28 return ok
29 }
30
31@@ -80,7 +85,9 @@
32 if err := s.ProcessFunctionHook(s, container, name); err != nil {
33 return nil, err
34 }
35+ s.mu.Lock()
36 data, ok := s.containers[container][name]
37+ s.mu.Unlock()
38 if !ok {
39 return nil, fmt.Errorf("no such object %q in container %q", name, container)
40 }
41@@ -96,7 +103,9 @@
42 if s.HasContainer(name) {
43 return fmt.Errorf("container already exists %q", name)
44 }
45+ s.mu.Lock()
46 s.containers[name] = make(object)
47+ s.mu.Unlock()
48 return nil
49 }
50
51@@ -110,7 +119,9 @@
52 if ok := s.HasContainer(name); !ok {
53 return nil, fmt.Errorf("no such container %q", name)
54 }
55+ s.mu.Lock()
56 items := s.containers[name]
57+ s.mu.Unlock()
58 sorted := make([]string, 0, len(items))
59 prefix := params["prefix"]
60 for filename := range items {
61@@ -154,7 +165,9 @@
62 return err
63 }
64 }
65+ s.mu.Lock()
66 s.containers[container][name] = data
67+ s.mu.Unlock()
68 return nil
69 }
70
71@@ -166,7 +179,9 @@
72 if ok := s.HasContainer(name); !ok {
73 return fmt.Errorf("no such container %q", name)
74 }
75+ s.mu.Lock()
76 delete(s.containers, name)
77+ s.mu.Unlock()
78 return nil
79 }
80
81@@ -178,7 +193,9 @@
82 if _, err := s.GetObject(container, name); err != nil {
83 return err
84 }
85+ s.mu.Lock()
86 delete(s.containers[container], name)
87+ s.mu.Unlock()
88 return nil
89 }
90

Subscribers

People subscribed via source and target branches