diff --git a/.github/emulator/start-emulator.sh b/.github/emulator/start-emulator.sh index 5f01d610..35c7eaa0 100755 --- a/.github/emulator/start-emulator.sh +++ b/.github/emulator/start-emulator.sh @@ -1,10 +1,10 @@ #!/bin/sh -eux -docker pull -q gcr.io/google.com/cloudsdktool/cloud-sdk:384.0.1-emulators +docker pull -q gcr.io/google.com/cloudsdktool/cloud-sdk:589.0.0-emulators CONTAINER=`docker run \ -d \ -p 8085:8085 \ - gcr.io/google.com/cloudsdktool/cloud-sdk:384.0.1-emulators gcloud beta emulators $1 start --host-port=0.0.0.0:8085 --project=dev` + gcr.io/google.com/cloudsdktool/cloud-sdk:589.0.0-emulators gcloud beta emulators $1 start --host-port=0.0.0.0:8085 --project=dev` until [ "`docker inspect -f {{.State.Running}} $CONTAINER`"=="true" ]; do sleep 0.1; diff --git a/plugins/dummy.txt b/plugins/dummy.txt new file mode 100644 index 00000000..aafa77a0 --- /dev/null +++ b/plugins/dummy.txt @@ -0,0 +1 @@ +trigger-workflow \ No newline at end of file diff --git a/treatment-service/models/storage.go b/treatment-service/models/storage.go index c94cf568..a9f77168 100644 --- a/treatment-service/models/storage.go +++ b/treatment-service/models/storage.go @@ -307,13 +307,18 @@ func (s *LocalStorage) findSubscribedProjectSettingsById(projectId ProjectId) *p return nil } - return s.findProjectSettingsById(projectId) + return s.findProjectSettingsByIdLocked(projectId) } func (s *LocalStorage) findProjectSettingsById(projectId ProjectId) *pubsub.ProjectSettings { s.RLock() defer s.RUnlock() + return s.findProjectSettingsByIdLocked(projectId) +} + +// findProjectSettingsByIdLocked looks up project settings; caller must hold s's read or write lock. +func (s *LocalStorage) findProjectSettingsByIdLocked(projectId ProjectId) *pubsub.ProjectSettings { for _, settings := range s.ProjectSettings { if ProjectId(settings.ProjectId) == projectId { return settings diff --git a/treatment-service/models/storage_test.go b/treatment-service/models/storage_test.go index 5b0a13ed..82627cc2 100644 --- a/treatment-service/models/storage_test.go +++ b/treatment-service/models/storage_test.go @@ -11,6 +11,7 @@ import ( "os" "reflect" "strings" + "sync" "testing" "time" @@ -735,3 +736,45 @@ func TestCustomSegmenter(t *testing.T) { {Key: segmenterName, Value: []*_segmenters.SegmenterValue{{Value: &_segmenters.SegmenterValue_String_{String_: "stringval"}}}}}) assert.Equal(t, 1, len(experimentmatch)) } + +// Readers keep the nested-lock path hot while a writer takes Lock(); a nested RLock would hang the writer. +func TestFindProjectSettingsWithId_ConcurrentWithWriter_NoDeadlock(t *testing.T) { + s := &LocalStorage{ + subscribedProjectIds: []ProjectId{1}, + ProjectSettings: []*_pubsub.ProjectSettings{{ProjectId: 1}}, + } + + stop := make(chan struct{}) + var readers sync.WaitGroup + for i := 0; i < 50; i++ { + readers.Add(1) + go func() { + defer readers.Done() + for { + select { + case <-stop: + return + default: + s.FindProjectSettingsWithId(1) + } + } + }() + } + + writerDone := make(chan struct{}) + go func() { + defer close(writerDone) + for i := 0; i < 2000; i++ { + s.UpdateProjectSettings(&_pubsub.ProjectSettings{ProjectId: 1}) + } + }() + + select { + case <-writerDone: + case <-time.After(5 * time.Second): + t.Fatal("deadlock: writer did not finish while readers called FindProjectSettingsWithId") + } + + close(stop) + readers.Wait() +}