race condition(s) around adding/deleting sessions

[adamdecaf]

Hey!

I was looking at this project, but noticed there might be some race conditions if your sqlitestore is used by multiple goroutines. Have you done so?

https://github.com/michaeljs1990/sqlitestore/blob/master/sqlitestore.go#L187-L189
https://github.com/gorilla/sessions/blob/master/sessions.go#L37

Those delete(.., ..) calls look to interact directly with a map[interface{}]interface{} without any locking.

[michaeljs1990]

Hey @adamdecaf,

Thanks for taking a look through this code. I have not used this with goroutines before this was mainly used by me as a slightly lighter way to do some integration testing for some code I had that used MySQL since I didn't need anything that was specific to MySQL itself SQLite worked great for testing. It does look like this could cause a race condition the exact one I believe described here golang/go#17922. Which as of go 1.8 will throw a panic which is undesirable.

I think it would be fairly easy to do some naive locking around this delete that should fix this issue however possibly taking a deeper look into sessions.go we could completely do away with the delete call but that is just be being optimistic.

tl;dr I have not used this in a goroutine and this will almost certainly throw a panic at some point if you run your application under any kind of load.

I looked at some other session backend adapters to see if I could lift something out quickly however they actually all seem to be doing this without locking from the ~4 I looked at including several MySQL and redis backends.

[adamdecaf]

tl;dr I have not used this in a goroutine and this will almost certainly throw a panic at some point if you run your application under any kind of load.

That's my assumption as well. I haven't decided on using gorilla/session, but if I do I'll PR this with some sync additions.

Thanks!