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

[windf17]

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.