1
0
mirror of https://github.com/gofiber/fiber.git synced 2025-04-27 13:14:31 +00:00

fix(middleware/session): fix data-race with sync.Pool ()

* feat: Add session mutex lock for thread safety

* chore: Refactor releaseSession mutex

* docs: Improve session.Save() function

The changes include updating the comments to provide clearer explanations of the function's behavior.
This commit is contained in:
Jason McNeil 2024-06-30 16:18:11 -03:00 committed by GitHub
parent 21ede5954c
commit 0400af6e47
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 21 additions and 6 deletions
middleware/session

@ -42,6 +42,7 @@ func acquireSession() *Session {
} }
func releaseSession(s *Session) { func releaseSession(s *Session) {
s.mu.Lock()
s.id = "" s.id = ""
s.exp = 0 s.exp = 0
s.ctx = nil s.ctx = nil
@ -52,6 +53,7 @@ func releaseSession(s *Session) {
if s.byteBuffer != nil { if s.byteBuffer != nil {
s.byteBuffer.Reset() s.byteBuffer.Reset()
} }
s.mu.Unlock()
sessionPool.Put(s) sessionPool.Put(s)
} }
@ -106,8 +108,8 @@ func (s *Session) Destroy() error {
// Reset local data // Reset local data
s.data.Reset() s.data.Reset()
s.mu.Lock() s.mu.RLock()
defer s.mu.Unlock() defer s.mu.RUnlock()
// Use external Storage if exist // Use external Storage if exist
if err := s.config.Storage.Delete(s.id); err != nil { if err := s.config.Storage.Delete(s.id); err != nil {
@ -141,6 +143,10 @@ func (s *Session) Reset() error {
if s.data != nil { if s.data != nil {
s.data.Reset() s.data.Reset()
} }
s.mu.Lock()
defer s.mu.Unlock()
// Reset byte buffer // Reset byte buffer
if s.byteBuffer != nil { if s.byteBuffer != nil {
s.byteBuffer.Reset() s.byteBuffer.Reset()
@ -148,9 +154,6 @@ func (s *Session) Reset() error {
// Reset expiration // Reset expiration
s.exp = 0 s.exp = 0
s.mu.Lock()
defer s.mu.Unlock()
// Delete old id from storage // Delete old id from storage
if err := s.config.Storage.Delete(s.id); err != nil { if err := s.config.Storage.Delete(s.id); err != nil {
return err return err
@ -172,6 +175,11 @@ func (s *Session) refresh() {
} }
// Save will update the storage and client cookie // Save will update the storage and client cookie
//
// sess.Save() will save the session data to the storage and update the
// client cookie, and it will release the session after saving.
//
// It's not safe to use the session after calling Save().
func (s *Session) Save() error { func (s *Session) Save() error {
// Better safe than sorry // Better safe than sorry
if s.data == nil { if s.data == nil {
@ -179,7 +187,6 @@ func (s *Session) Save() error {
} }
s.mu.Lock() s.mu.Lock()
defer s.mu.Unlock()
// Check if session has your own expiration, otherwise use default value // Check if session has your own expiration, otherwise use default value
if s.exp <= 0 { if s.exp <= 0 {
@ -205,6 +212,8 @@ func (s *Session) Save() error {
return err return err
} }
s.mu.Unlock()
// Release session // Release session
// TODO: It's not safe to use the Session after calling Save() // TODO: It's not safe to use the Session after calling Save()
releaseSession(s) releaseSession(s)

@ -77,6 +77,10 @@ func (s *Store) Get(c fiber.Ctx) (*Session, error) {
// Create session object // Create session object
sess := acquireSession() sess := acquireSession()
sess.mu.Lock()
defer sess.mu.Unlock()
sess.ctx = c sess.ctx = c
sess.config = s sess.config = s
sess.id = id sess.id = id
@ -84,6 +88,8 @@ func (s *Store) Get(c fiber.Ctx) (*Session, error) {
// Decode session data if found // Decode session data if found
if rawData != nil { if rawData != nil {
sess.data.Lock()
defer sess.data.Unlock()
if err := sess.decodeSessionData(rawData); err != nil { if err := sess.decodeSessionData(rawData); err != nil {
return nil, fmt.Errorf("failed to decode session data: %w", err) return nil, fmt.Errorf("failed to decode session data: %w", err)
} }