Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix missing initial sync.RWMutex #2305

Merged
merged 3 commits into from Mar 27, 2020
Merged

fix missing initial sync.RWMutex #2305

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ba621af
fix missing initial sync.RWMutex
appleboy Mar 27, 2020
fdebef2
Add unit testing.
appleboy Mar 27, 2020
1bb0456
Merge branch 'master' into bugs2
appleboy Mar 27, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 8 additions & 0 deletions context.go
View file
Open in desktop
Expand Up @@ -224,6 +224,10 @@ func (c *Context) Error(err error) *Error {
// Set is used to store a new key/value pair exclusively for this context.
// It also lazy initializes c.Keys if it was not used previously.
func (c *Context) Set(key string, value interface{}) {
if c.KeysMutex == nil {
c.KeysMutex = &sync.RWMutex{}
}
Copy link

@felix-h felix-h Mar 30, 2020 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@appleboy @thinkerou Isn't the mutex creation still a race condition? For two threads that are both calling "Set" at the same time, the mutex could get created twice -- once for each thread. What about using "sync.once" here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@felix-h fix in #2351


c.KeysMutex.Lock()
if c.Keys == nil {
c.Keys = make(map[string]interface{})
Expand All @@ -236,6 +240,10 @@ func (c *Context) Set(key string, value interface{}) {
// Get returns the value for the given key, ie: (value, true).
// If the value does not exists it returns (nil, false)
func (c *Context) Get(key string) (value interface{}, exists bool) {
if c.KeysMutex == nil {
c.KeysMutex = &sync.RWMutex{}
}

c.KeysMutex.RLock()
value, exists = c.Keys[key]
c.KeysMutex.RUnlock()
Expand Down
13 changes: 13 additions & 0 deletions context_test.go
View file
Open in desktop
Expand Up @@ -1918,3 +1918,16 @@ func TestRaceParamsContextCopy(t *testing.T) {
performRequest(router, "GET", "/name2/api")
wg.Wait()
}

func TestContextWithKeysMutex(t *testing.T) {
c := &Context{}
c.Set("foo", "bar")

value, err := c.Get("foo")
assert.Equal(t, "bar", value)
assert.True(t, err)

value, err = c.Get("foo2")
assert.Nil(t, value)
assert.False(t, err)
}
1 change: 1 addition & 0 deletions go.sum
View file
Open in desktop
@@ -1,4 +1,5 @@
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/gin-contrib/sse v0.1.0 h1:Y/yl/+YNO8GZSjAhjMsSuLt29uWRFHdHYUb5lYOV9qE=
github.com/gin-contrib/sse v0.1.0/go.mod h1:RHrZQHXnP2xjPF+u1gW/2HnVO7nvIa9PG3Gm+fLHvGI=
Expand Down