Hacktoberfest 2026: the issues maintainers tagged for October, open and beginner-friendly. Browse Hacktoberfest issues

[gsession] StorageFile.SetSession 非原子写入导致并发读请求看到空 session

Open Beginner friendly
  • Is something wrong?
#4,792 0 comments 0 reactions 0 assignees View on GitHub

Maintainers usually reply within 2 days

Nobody has claimed this yet.

  • #4814 by @waterWang — closed without merging

Assessment

Difficulty
2/5
Estimated time
1-3 hours
Newbie friendliness
74/100
Issue type
Bug
Clarity
Clearly specified
Activity status
Quiet
Tech stack
go
Domain
backend

Research direction

Read the two paths in gsession_storage_file.go: SetSession around lines 189-203 (O_TRUNC open then two separate Write calls) and GetSession at 141-170, which treats content of 8 bytes or fewer as an empty session. Reproduce the race first with a test that calls GetSession concurrently while SetSession rewrites the same session ID, and keep the 8-byte gtime.TimestampMilli() prefix format intact. Check the closed, unmerged PR #4814 for a prior attempt before starting. Done means SetSession writes a temp file and renames it atomically, with existing gsession storage tests still passing.

Written by the indexing model from the issue text.

Description

Description

StorageFile.SetSession writes session data using a non-atomic file write pattern (open with O_TRUNC then two separate Write calls), creating a race window where concurrent readers see an empty or partially-written file.

Impact

Under concurrent requests sharing the same session ID (common in SPA applications where page switching fires multiple API calls simultaneously), intermittent authentication failures occur:

  • Session data is lost; GetSession returns nil
  • User appears "not logged in" and gets redirected to login

Root Cause

Write path (gsession_storage_file.go:189-203)
file, err := gfile.OpenWithFlagPerm(
    path, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, os.ModePerm,
)
// ...
file.Write(gbinary.EncodeInt64(gtime.TimestampMilli()))  // 8 bytes timestamp
file.Write(content)                                         // JSON body

O_TRUNC empties the file immediately. Then two separate file.Write() calls create a real gap between writing the timestamp header and the JSON body.

Read path (gsession_storage_file.go:141-170)
content = gfile.GetBytes(path)
if len(content) > 8 {
    // parse JSON
}
return nil, nil  // content <= 8 bytes → empty session
Race window
Writer(SetSession)                  Reader(GetSession)
│                                    │
├─O_TRUNC open → file is empty      │
│                                    ├─gfile.GetBytes() → 0-8 bytes
│                                    │ len<=8 → returns nil session
│                                    │ → user appears not logged in!
├─file.Write(timestamp)[8 bytes]    │
├─file.Write(content)[JSON body]    │
│ write complete                     │

Production Evidence

Real log (3ms window, same session ID):

12:16:47.467  devices/all          userId=1  ← normal
12:16:47.470  software-systems/all userId=0  ← user_id key missing! (race hit)
12:16:47.472  network-lines/all    userId=1  ← normal

Proposed Fix: atomic write via temp file + rename

func (s *StorageFile) SetSession(ctx context.Context, sessionId string, sessionData *gmap.StrAnyMap, ttl time.Duration) error {
    intlog.Printf(ctx, "StorageFile.SetSession: %s, %v, %v", sessionId, sessionData, ttl)
    path := s.sessionFilePath(sessionId)
    content, err := json.Marshal(sessionData)
    if err != nil {
        return err
    }
    if s.cryptoEnabled {
        content, err = gaes.Encrypt(content, DefaultStorageFileCryptoKey)
        if err != nil {
            return err
        }
    }

    // Build content with 8-byte timestamp prefix (same format)
    timestamp := gbinary.EncodeInt64(gtime.TimestampMilli())
    data := make([]byte, 8+len(content))
    copy(data[:8], timestamp)
    copy(data[8:], content)

    // Step 1: write to temp file (invisible to readers)
    tmpPath := path + ".tmp"
    if err := os.WriteFile(tmpPath, data, os.ModePerm); err != nil {
        return gerror.Wrapf(err, `write data failed to file "%s"`, tmpPath)
    }

    // Step 2: atomic rename (appears atomically to readers)
    if err := os.Rename(tmpPath, path); err != nil {
        os.Remove(tmpPath)
        return gerror.Wrapf(err, `rename "%s" -> "%s" failed`, tmpPath, path)
    }
    return nil
}

os.Rename is atomic on the same filesystem (POSIX guarantee), so readers always see a complete file.

Dominant language
Go
Stars
13.3k
Forks
1.7k
Avg merge
4d 11h
Merged PRs (30d)
42

Getting set up

This project ships no dev container, Dockerfile or contributing guide, so setting up is up to you: start from its README, and see our first-contribution guide for the general steps.

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from gogf/gf

All issues in gogf/gf

Similar issues

More Go issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.