Skip to content

Commit 0400af6

Browse files
authored
fix(middleware/session): fix data-race with sync.Pool (#3051)
* 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.
1 parent 21ede59 commit 0400af6

File tree

2 files changed

+21
-6
lines changed

2 files changed

+21
-6
lines changed

middleware/session/session.go

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ func acquireSession() *Session {
4242
}
4343

4444
func releaseSession(s *Session) {
45+
s.mu.Lock()
4546
s.id = ""
4647
s.exp = 0
4748
s.ctx = nil
@@ -52,6 +53,7 @@ func releaseSession(s *Session) {
5253
if s.byteBuffer != nil {
5354
s.byteBuffer.Reset()
5455
}
56+
s.mu.Unlock()
5557
sessionPool.Put(s)
5658
}
5759

@@ -106,8 +108,8 @@ func (s *Session) Destroy() error {
106108
// Reset local data
107109
s.data.Reset()
108110

109-
s.mu.Lock()
110-
defer s.mu.Unlock()
111+
s.mu.RLock()
112+
defer s.mu.RUnlock()
111113

112114
// Use external Storage if exist
113115
if err := s.config.Storage.Delete(s.id); err != nil {
@@ -141,16 +143,17 @@ func (s *Session) Reset() error {
141143
if s.data != nil {
142144
s.data.Reset()
143145
}
146+
147+
s.mu.Lock()
148+
defer s.mu.Unlock()
149+
144150
// Reset byte buffer
145151
if s.byteBuffer != nil {
146152
s.byteBuffer.Reset()
147153
}
148154
// Reset expiration
149155
s.exp = 0
150156

151-
s.mu.Lock()
152-
defer s.mu.Unlock()
153-
154157
// Delete old id from storage
155158
if err := s.config.Storage.Delete(s.id); err != nil {
156159
return err
@@ -172,14 +175,18 @@ func (s *Session) refresh() {
172175
}
173176

174177
// Save will update the storage and client cookie
178+
//
179+
// sess.Save() will save the session data to the storage and update the
180+
// client cookie, and it will release the session after saving.
181+
//
182+
// It's not safe to use the session after calling Save().
175183
func (s *Session) Save() error {
176184
// Better safe than sorry
177185
if s.data == nil {
178186
return nil
179187
}
180188

181189
s.mu.Lock()
182-
defer s.mu.Unlock()
183190

184191
// Check if session has your own expiration, otherwise use default value
185192
if s.exp <= 0 {
@@ -205,6 +212,8 @@ func (s *Session) Save() error {
205212
return err
206213
}
207214

215+
s.mu.Unlock()
216+
208217
// Release session
209218
// TODO: It's not safe to use the Session after calling Save()
210219
releaseSession(s)

middleware/session/store.go

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,13 +77,19 @@ func (s *Store) Get(c fiber.Ctx) (*Session, error) {
7777

7878
// Create session object
7979
sess := acquireSession()
80+
81+
sess.mu.Lock()
82+
defer sess.mu.Unlock()
83+
8084
sess.ctx = c
8185
sess.config = s
8286
sess.id = id
8387
sess.fresh = fresh
8488

8589
// Decode session data if found
8690
if rawData != nil {
91+
sess.data.Lock()
92+
defer sess.data.Unlock()
8793
if err := sess.decodeSessionData(rawData); err != nil {
8894
return nil, fmt.Errorf("failed to decode session data: %w", err)
8995
}

0 commit comments

Comments
 (0)