Gin: "concurrent map read and map write" in getting context

[ntquyen]

I got a race condition when getting value from Context: concurrent map read and map write

The potential error code is in /gin-gonic/gin/context.go

// Set is used to store a new key/value pair exclusivelly 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.Keys == nil {
        c.Keys = make(map[string]interface{})
    }
    c.Keys[key] = value
}

// 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.Keys != nil {
        value, exists = c.Keys[key]
    }
    return
}

I thought here we must use Mutex around c.Keys[key] to avoid the race condition.

[crazbot]

is the Context often used as a container or something else?

[ntquyen]

Yes, it is used as a map to store header values and some request metrics

[crazbot]

okay, uh, so the Context is commonly used as a single entity. well, we should use Mutex in our logic instead of directly bind a Mutex to the Contexts.

[stxml]

Should we orient ourselves toward context.Context? It is safe to use concurrently and gin.Context implements the interface.

[NeoCN]

any update on this issue?

[maxatome]

As I just commented in pull request #702, putting a Mutex around c.Keys is a bad idea due to the recycling logic gin gonic applies to Context instances.
You have to do a copy of the context (or of the Keys map) before sharing it, then handle the locking mechanism at your level.

[NeoCN]

Because of the recycling logic gin applies to Context, we can not simply use gin.Context and must do c.Copy for continue use(may be used in a goroutine), and this result that we do c.Copy nearly almost every method. Just thinking about we need an option to disable recycling logic for gin.Context.

[themartorana]

Not sure if anyone started getting this issue with Go 1.8, but we did - a lot. (We just deployed to production with 1.8 for the first time today.)

Looks like 1.8 added a new check. From https://stackoverflow.com/questions/42453442/concurrent-map-iteration-and-map-write-error-in-golang-1-8 :

There is a enhanced concurrent access check added in the Golang 1.8, and this is the source code in runtime/hashmap.go:736,

if h.flags&hashWriting != 0 {
    throw("concurrent map iteration and map write")
}

I'm trying to work around it right now, but I may be forced to toss a mutex lock into our vendor copy of Gin in the meantime (or roll back to 1.7)...

[rkuska]

FYI This issue should be closed gin.Context has a RWMutex for safeguarding since v1.6.0.

[niboge]

FYI This issue should be closed gin.Context has a RWMutex for safeguarding since v1.6.0.

set header 、 set response ， is it safe too?