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.

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

got panic with RWMutex on 1.6.0 version #2301

Closed
congtranminh opened this issue Mar 26, 2020 · 7 comments · Fixed by #2305
Closed

got panic with RWMutex on 1.6.0 version #2301

congtranminh opened this issue Mar 26, 2020 · 7 comments · Fixed by #2305
Assignees
Labels
Milestone

Comments

@congtranminh
Copy link

congtranminh commented Mar 26, 2020

  • With issues:

Description

After upgrade to 1.6.0, I got panic when use Get Set context.

panic: runtime error: invalid memory address or nil pointer dereference [recovered]
 	panic: runtime error: invalid memory address or nil pointer dereference
 [signal SIGSEGV: segmentation violation code=0x1 addr=0x10 pc=0x9193c3]
 goroutine 9 [running]:
 testing.tRunner.func1(0xc000137100)
 	/usr/local/go/src/testing/testing.go:830 +0x392
 panic(0xb33d40, 0x120e870)
 	/usr/local/go/src/runtime/panic.go:522 +0x1b5
 sync.(*RWMutex).RLock(...)
 	/usr/local/go/src/sync/rwmutex.go:48
 github.com/gin-gonic/gin.(*Context).Get(0xc0003b8000, 0xbfc6f8, 0x5, 0x2, 0x2, 0x10)
 	/go/pkg/mod/github.com/gin-gonic/gin@v1.6.1/context.go:239 +0x33

I compare with the 1.5.0 version, and saw that you used RWMutex inside the Get context function.
Please check and remove it as 1.5.0 version.

// 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) {
	c.KeysMutex.RLock()
	value, exists = c.Keys[key]
	c.KeysMutex.RUnlock()
	return
}

How to reproduce

Here my code example

type logger struct {
	GinCtx  *gin.Context
}

func (_this *logger) Set(key string, value interface{}) {
	if _this.GinCtx != nil {
		_this.GinCtx.Set(key, value)
	}
}

func (_this *logger) Get(key string) interface{} {
	if _this.GinCtx != nil {
		value, _ := _this.GinCtx.Get(key)
		return value
	}
	return nil
}

on handler package, I call this Get, Set function and got panic :(

Environment

  • go version: 1.13
  • gin version (or commit ref): 1.6.0, 1.6.1
  • operating system: linux
@congtranminh congtranminh changed the title got panic with 1.6.0 version got panic with RWMutex on 1.6.0 version Mar 26, 2020
@jdolitsky
Copy link

Same issue here using Gin 1.6.1, Go 1.14.1, Mac OS. Downgrading to Gin 1.5.0 fixes the issue.

@appleboy
Copy link
Member

appleboy commented Mar 27, 2020

cc @AcoNCodes ref: #1391

@appleboy appleboy added this to the 1.6.2 milestone Mar 27, 2020
@appleboy
Copy link
Member

fix in #2305 please help to confirm PR.

@jdolitsky
Copy link

@appleboy - confirmed. Fixes things on my end.

@appleboy
Copy link
Member

appleboy commented Mar 27, 2020

Also add unit testing:

func TestContextWithKeysMutex(t *testing.T) {
	c := &gin.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)
}

@thinkerou Please help to review PR.

@appleboy
Copy link
Member

bump to v1.6.2 https://github.com/gin-gonic/gin/releases/tag/v1.6.2

@congtranminh
Copy link
Author

thank you guys.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants