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

Session Keys and access methods Get and Set is not concurrency safe #71

Closed
kokteyldev opened this issue Sep 28, 2022 · 3 comments
Closed
Labels

Comments

@kokteyldev
Copy link

kokteyldev commented Sep 28, 2022

Hi,

We just received the following error in our server:

fatal error: concurrent map read and map write

goroutine 900594 [running]:
runtime.throw(0xff43c6, 0x21)
	/usr/local/go/src/runtime/panic.go:1116 +0x72 fp=0xc0050ad860 sp=0xc0050ad830 pc=0x437c52
runtime.mapaccess2_faststr(0xe996c0, 0xc005367290, 0xfd8f40, 0x5, 0x0, 0x0)
	/usr/local/go/src/runtime/map_faststr.go:116 +0x4a5 fp=0xc0050ad8d0 sp=0xc0050ad860 pc=0x415de5
gopkg.in/olahol/melody%2ev1.(*Session).Get(...)
	/home/erdem/Documents/gopath/code/pkg/mod/gopkg.in/olahol/melody.v1@v1.0.0-20170518105555-d52139073376/session.go:201

When we checked it seems like the rwmutex in the session struct is not used to protect the Keys map inside it but only used to set and get the closed state. I couldn't figure out if this is deliberate and the consumer of the package is supposed to protect the Keys access with their own mutex. If it is then maybe a two new access methods can be added like SafeGet and SafeSet that uses the session rw mutex to protect access.

I can submit a pull request if you like for this issue.

@kokteyldev
Copy link
Author

kokteyldev commented Sep 28, 2022

This is the same issue as #42 but I think it is not merged yet, do you have any plans to merge the PR

@olahol
Copy link
Owner

olahol commented Sep 28, 2022

Yes, I will be adding some tests and and publishing a new version this week.

@olahol olahol added the bug label Sep 28, 2022
@olahol
Copy link
Owner

olahol commented Oct 9, 2022

Fixed in v1.1.1

@olahol olahol closed this as completed Oct 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants