fix(racy p2p tests): don't pass locks by value in MemStore

thankfully go vet caught this for us, should help reduce race conditions that keep showing up in tests. more info: https://medium.com/golangspec/detect-locks-passed-by-value-in-go-efb4ac9a3f2b
qri-io · Oct 30, 2018 · 5e410a3 · 5e410a3
1 parent e0a4512
commit 5e410a3
Show file tree

Hide file tree

Showing 2 changed files with 10 additions and 10 deletions.
diff --git a/actions/merge_events_test.go b/actions/merge_events_test.go
@@ -24,11 +24,11 @@ func createReposAndLogs() (repo.Repo, repo.Repo, *repo.MemEventLog, *repo.MemEve
  aRepo, _ := repo.NewMemRepo(&profile.Profile{
  ID: profile.ID(profileAID),
  Peername: "test-peer-0",
- }, cafs.NewMapstore(), profile.MemStore{}, nil)
+ }, cafs.NewMapstore(), &profile.MemStore{}, nil)
  bRepo, _ := repo.NewMemRepo(&profile.Profile{
  ID: profile.ID(profileBID),
  Peername: "test-peer-0",
- }, cafs.NewMapstore(), profile.MemStore{}, nil)
+ }, cafs.NewMapstore(), &profile.MemStore{}, nil)
  aLog := aRepo.MemEventLog
  bLog := bRepo.MemEventLog
  return aRepo, bRepo, aLog, bLog

diff --git a/repo/profile/store.go b/repo/profile/store.go
@@ -23,7 +23,7 @@ type Store interface {
 
 // MemStore is an in-memory implementation of the profile Store interface
 type MemStore struct {
- sync.RWMutex
+ sync.Mutex
  store map[ID]*Profile
 }
 
@@ -35,7 +35,7 @@ func NewMemStore() Store {
 }
 
 // PutProfile adds a peer to this store
-func (m MemStore) PutProfile(profile *Profile) error {
+func (m *MemStore) PutProfile(profile *Profile) error {
  if profile.ID.String() == "" {
  return fmt.Errorf("profile.ID is required")
  }
@@ -47,7 +47,7 @@ func (m MemStore) PutProfile(profile *Profile) error {
 }
 
 // PeernameID gives the ID for a given peername
-func (m MemStore) PeernameID(peername string) (ID, error) {
+func (m *MemStore) PeernameID(peername string) (ID, error) {
  m.Lock()
  defer m.Unlock()
 
@@ -62,7 +62,7 @@ func (m MemStore) PeernameID(peername string) (ID, error) {
 // PeerProfile returns profile data for a given peer.ID
 // TODO - this func implies that peer.ID's are only ever connected to the same
 // profile. That could cause trouble.
-func (m MemStore) PeerProfile(id peer.ID) (*Profile, error) {
+func (m *MemStore) PeerProfile(id peer.ID) (*Profile, error) {
  m.Lock()
  defer m.Unlock()
 
@@ -80,7 +80,7 @@ func (m MemStore) PeerProfile(id peer.ID) (*Profile, error) {
 }
 
 // PeerIDs gives the peer.IDs list for a given peername
-func (m MemStore) PeerIDs(id ID) ([]peer.ID, error) {
+func (m *MemStore) PeerIDs(id ID) ([]peer.ID, error) {
  m.Lock()
  defer m.Unlock()
 
@@ -94,7 +94,7 @@ func (m MemStore) PeerIDs(id ID) ([]peer.ID, error) {
 }
 
 // List hands the full list of peers back
-func (m MemStore) List() (map[ID]*Profile, error) {
+func (m *MemStore) List() (map[ID]*Profile, error) {
  m.Lock()
  defer m.Unlock()
 
@@ -106,7 +106,7 @@ func (m MemStore) List() (map[ID]*Profile, error) {
 }
 
 // GetProfile give's peer info from the store for a given peer.ID
-func (m MemStore) GetProfile(id ID) (*Profile, error) {
+func (m *MemStore) GetProfile(id ID) (*Profile, error) {
  m.Lock()
  defer m.Unlock()
 
@@ -117,7 +117,7 @@ func (m MemStore) GetProfile(id ID) (*Profile, error) {
 }
 
 // DeleteProfile removes a peer from this store
-func (m MemStore) DeleteProfile(id ID) error {
+func (m *MemStore) DeleteProfile(id ID) error {
  m.Lock()
  delete(m.store, id)
  m.Unlock()