search

olahol / melody

:notes: Minimalist websocket framework for Go
BSD 2-Clause "Simplified" License
3.75k stars 366 forks source link

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

Closed kokteyldev closed 2 years ago

kokteyldev commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

olahol commented 2 years ago

Fixed in v1.1.1