fix(snapshot): bound repository session state
- commit
- 7e93b8d61df7afffba560911e6c3c7021055b50d
- author
- brice · · 2026-07-22 02:56 UTC
- parent
- 70f8dc3
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
10 files · +260 -100
M internal/adapters/snapshot/legacy.go +6 -9
| @@ -122,9 +122,8 @@ func (r *Repository) DeleteLegacy(ctx context.Context, name string) error { | |||
| 122 | 122 | return err | |
| 123 | 123 | } | |
| 124 | 124 | filename := filenameForName(name) | |
| 125 | - | lock := r.sessionLock("legacy-sync:" + filename) | |
| 126 | - | lock.Lock() | |
| 127 | - | defer lock.Unlock() | |
| 125 | + | lock := r.lockSession("legacy-sync:" + filename) | |
| 126 | + | defer r.unlockSession(lock) | |
| 128 | 127 | if err := ctx.Err(); err != nil { | |
| 129 | 128 | return err | |
| 130 | 129 | } | |
| @@ -252,9 +251,8 @@ func (r *Repository) Tombstone(ctx context.Context, name string) error { | |||
| 252 | 251 | return err | |
| 253 | 252 | } | |
| 254 | 253 | key := sessionKey(name) | |
| 255 | - | lock := r.sessionLock(key) | |
| 256 | - | lock.Lock() | |
| 257 | - | defer lock.Unlock() | |
| 254 | + | lock := r.lockSession(key) | |
| 255 | + | defer r.unlockSession(lock) | |
| 258 | 256 | if err := ctx.Err(); err != nil { | |
| 259 | 257 | return err | |
| 260 | 258 | } | |
| @@ -280,9 +278,8 @@ func (r *Repository) DeleteTombstone(ctx context.Context, name string) error { | |||
| 280 | 278 | return err | |
| 281 | 279 | } | |
| 282 | 280 | key := sessionKey(name) | |
| 283 | - | lock := r.sessionLock(key) | |
| 284 | - | lock.Lock() | |
| 285 | - | defer lock.Unlock() | |
| 281 | + | lock := r.lockSession(key) | |
| 282 | + | defer r.unlockSession(lock) | |
| 286 | 283 | if err := ctx.Err(); err != nil { | |
| 287 | 284 | return err | |
| 288 | 285 | } | |
| @@ -122,9 +122,8 @@ func (r *Repository) DeleteLegacy(ctx context.Context, name string) error { | |||||
| 122 | return err | 122 | return err | ||
| 123 | } | 123 | } | ||
| 124 | filename := filenameForName(name) | 124 | filename := filenameForName(name) | ||
| 125 | - | lock := r.sessionLock("legacy-sync:" + filename) | 125 | + | lock := r.lockSession("legacy-sync:" + filename) |
| 126 | - | lock.Lock() | 126 | + | defer r.unlockSession(lock) |
| 127 | - | defer lock.Unlock() | |||
| 128 | if err := ctx.Err(); err != nil { | 127 | if err := ctx.Err(); err != nil { | ||
| 129 | return err | 128 | return err | ||
| 130 | } | 129 | } | ||
| @@ -252,9 +251,8 @@ func (r *Repository) Tombstone(ctx context.Context, name string) error { | |||||
| 252 | return err | 251 | return err | ||
| 253 | } | 252 | } | ||
| 254 | key := sessionKey(name) | 253 | key := sessionKey(name) | ||
| 255 | - | lock := r.sessionLock(key) | 254 | + | lock := r.lockSession(key) |
| 256 | - | lock.Lock() | 255 | + | defer r.unlockSession(lock) |
| 257 | - | defer lock.Unlock() | |||
| 258 | if err := ctx.Err(); err != nil { | 256 | if err := ctx.Err(); err != nil { | ||
| 259 | return err | 257 | return err | ||
| 260 | } | 258 | } | ||
| @@ -280,9 +278,8 @@ func (r *Repository) DeleteTombstone(ctx context.Context, name string) error { | |||||
| 280 | return err | 278 | return err | ||
| 281 | } | 279 | } | ||
| 282 | key := sessionKey(name) | 280 | key := sessionKey(name) | ||
| 283 | - | lock := r.sessionLock(key) | 281 | + | lock := r.lockSession(key) |
| 284 | - | lock.Lock() | 282 | + | defer r.unlockSession(lock) |
| 285 | - | defer lock.Unlock() | |||
| 286 | if err := ctx.Err(); err != nil { | 283 | if err := ctx.Err(); err != nil { | ||
| 287 | return err | 284 | return err | ||
| 288 | } | 285 | } | ||
M internal/adapters/snapshot/load.go +2 -3
| @@ -65,9 +65,8 @@ func (r *Repository) Load(ctx context.Context, name string) (ports.SnapshotGener | |||
| 65 | 65 | if killed { | |
| 66 | 66 | return ports.SnapshotGeneration{}, fmt.Errorf("snapshot session %q is killed", name) | |
| 67 | 67 | } | |
| 68 | - | lock := r.sessionLock(key) | |
| 69 | - | lock.Lock() | |
| 70 | - | defer lock.Unlock() | |
| 68 | + | lock := r.lockSession(key) | |
| 69 | + | defer r.unlockSession(lock) | |
| 71 | 70 | if err := ctx.Err(); err != nil { | |
| 72 | 71 | return ports.SnapshotGeneration{}, err | |
| 73 | 72 | } | |
| @@ -65,9 +65,8 @@ func (r *Repository) Load(ctx context.Context, name string) (ports.SnapshotGener | |||||
| 65 | if killed { | 65 | if killed { | ||
| 66 | return ports.SnapshotGeneration{}, fmt.Errorf("snapshot session %q is killed", name) | 66 | return ports.SnapshotGeneration{}, fmt.Errorf("snapshot session %q is killed", name) | ||
| 67 | } | 67 | } | ||
| 68 | - | lock := r.sessionLock(key) | 68 | + | lock := r.lockSession(key) |
| 69 | - | lock.Lock() | 69 | + | defer r.unlockSession(lock) |
| 70 | - | defer lock.Unlock() | |||
| 71 | if err := ctx.Err(); err != nil { | 70 | if err := ctx.Err(); err != nil { | ||
| 72 | return ports.SnapshotGeneration{}, err | 71 | return ports.SnapshotGeneration{}, err | ||
| 73 | } | 72 | } | ||
M internal/adapters/snapshot/maintenance.go +14 -3
| @@ -42,6 +42,15 @@ func (r *Repository) Maintain(ctx context.Context) (err error) { | |||
| 42 | 42 | if err := r.removeTemps(ctx, r.dir, &budget, "root-temps"); err != nil || budget == 0 { | |
| 43 | 43 | return err | |
| 44 | 44 | } | |
| 45 | + | // Resume an incomplete session before discovering another one. This keeps | |
| 46 | + | // retained marks (and their session lock/epoch references) bounded even | |
| 47 | + | // when a hostile repository contains arbitrarily many partial sessions. | |
| 48 | + | for key := range r.maintenanceSessions { | |
| 49 | + | lock := r.lockSession(key) | |
| 50 | + | err = r.maintainSession(ctx, key, &budget) | |
| 51 | + | r.unlockSession(lock) | |
| 52 | + | return err | |
| 53 | + | } | |
| 45 | 54 | sessions := filepath.Join(r.dir, repositorySessionsDir) | |
| 46 | 55 | entries, _, err := r.readMaintenanceDir(sessions, maintenanceBatch, "sessions") | |
| 47 | 56 | if errors.Is(err, os.ErrNotExist) { | |
| @@ -77,13 +86,15 @@ func (r *Repository) Maintain(ctx context.Context) (err error) { | |||
| 77 | 86 | if !entry.isDir || !canonicalSessionKey(entry.name) { | |
| 78 | 87 | continue | |
| 79 | 88 | } | |
| 80 | - | lock := r.sessionLock(entry.name) | |
| 81 | - | lock.Lock() | |
| 89 | + | lock := r.lockSession(entry.name) | |
| 82 | 90 | err = r.maintainSession(ctx, entry.name, &budget) | |
| 83 | - | lock.Unlock() | |
| 91 | + | r.unlockSession(lock) | |
| 84 | 92 | if err != nil { | |
| 85 | 93 | return err | |
| 86 | 94 | } | |
| 95 | + | if r.maintenanceSessions[entry.name] != nil { | |
| 96 | + | return nil | |
| 97 | + | } | |
| 87 | 98 | } | |
| 88 | 99 | return nil | |
| 89 | 100 | } | |
| @@ -42,6 +42,15 @@ func (r *Repository) Maintain(ctx context.Context) (err error) { | |||||
| 42 | if err := r.removeTemps(ctx, r.dir, &budget, "root-temps"); err != nil || budget == 0 { | 42 | if err := r.removeTemps(ctx, r.dir, &budget, "root-temps"); err != nil || budget == 0 { | ||
| 43 | return err | 43 | return err | ||
| 44 | } | 44 | } | ||
| 45 | + | // Resume an incomplete session before discovering another one. This keeps | |||
| 46 | + | // retained marks (and their session lock/epoch references) bounded even | |||
| 47 | + | // when a hostile repository contains arbitrarily many partial sessions. | |||
| 48 | + | for key := range r.maintenanceSessions { | |||
| 49 | + | lock := r.lockSession(key) | |||
| 50 | + | err = r.maintainSession(ctx, key, &budget) | |||
| 51 | + | r.unlockSession(lock) | |||
| 52 | + | return err | |||
| 53 | + | } | |||
| 45 | sessions := filepath.Join(r.dir, repositorySessionsDir) | 54 | sessions := filepath.Join(r.dir, repositorySessionsDir) | ||
| 46 | entries, _, err := r.readMaintenanceDir(sessions, maintenanceBatch, "sessions") | 55 | entries, _, err := r.readMaintenanceDir(sessions, maintenanceBatch, "sessions") | ||
| 47 | if errors.Is(err, os.ErrNotExist) { | 56 | if errors.Is(err, os.ErrNotExist) { | ||
| @@ -77,13 +86,15 @@ func (r *Repository) Maintain(ctx context.Context) (err error) { | |||||
| 77 | if !entry.isDir || !canonicalSessionKey(entry.name) { | 86 | if !entry.isDir || !canonicalSessionKey(entry.name) { | ||
| 78 | continue | 87 | continue | ||
| 79 | } | 88 | } | ||
| 80 | - | lock := r.sessionLock(entry.name) | 89 | + | lock := r.lockSession(entry.name) |
| 81 | - | lock.Lock() | |||
| 82 | err = r.maintainSession(ctx, entry.name, &budget) | 90 | err = r.maintainSession(ctx, entry.name, &budget) | ||
| 83 | - | lock.Unlock() | 91 | + | r.unlockSession(lock) |
| 84 | if err != nil { | 92 | if err != nil { | ||
| 85 | return err | 93 | return err | ||
| 86 | } | 94 | } | ||
| 95 | + | if r.maintenanceSessions[entry.name] != nil { | |||
| 96 | + | return nil | |||
| 97 | + | } | |||
| 87 | } | 98 | } | ||
| 88 | return nil | 99 | return nil | ||
| 89 | } | 100 | } | ||
M internal/adapters/snapshot/maintenance_cursor.go +3 -0
| @@ -24,6 +24,9 @@ type maintenanceDirEntry struct { | |||
| 24 | 24 | } | |
| 25 | 25 | ||
| 26 | 26 | func (r *Repository) resetMaintenance() { | |
| 27 | + | for key := range r.maintenanceSessions { | |
| 28 | + | r.clearSessionMaintenance(key) | |
| 29 | + | } | |
| 27 | 30 | r.maintenanceCursors = nil | |
| 28 | 31 | r.maintenanceSessions = nil | |
| 29 | 32 | } | |
| @@ -24,6 +24,9 @@ type maintenanceDirEntry struct { | |||||
| 24 | } | 24 | } | ||
| 25 | 25 | ||||
| 26 | func (r *Repository) resetMaintenance() { | 26 | func (r *Repository) resetMaintenance() { | ||
| 27 | + | for key := range r.maintenanceSessions { | |||
| 28 | + | r.clearSessionMaintenance(key) | |||
| 29 | + | } | |||
| 27 | r.maintenanceCursors = nil | 30 | r.maintenanceCursors = nil | ||
| 28 | r.maintenanceSessions = nil | 31 | r.maintenanceSessions = nil | ||
| 29 | } | 32 | } | ||
M internal/adapters/snapshot/maintenance_delete.go +18 -37
| @@ -8,7 +8,6 @@ import ( | |||
| 8 | 8 | "os" | |
| 9 | 9 | "path/filepath" | |
| 10 | 10 | "strings" | |
| 11 | - | "time" | |
| 12 | 11 | ) | |
| 13 | 12 | ||
| 14 | 13 | // Delete makes a session unavailable by durably moving it out of the canonical | |
| @@ -19,9 +18,8 @@ func (r *Repository) Delete(ctx context.Context, name string) error { | |||
| 19 | 18 | return err | |
| 20 | 19 | } | |
| 21 | 20 | key := sessionKey(name) | |
| 22 | - | lock := r.sessionLock(key) | |
| 23 | - | lock.Lock() | |
| 24 | - | defer lock.Unlock() | |
| 21 | + | lock := r.lockSession(key) | |
| 22 | + | defer r.unlockSession(lock) | |
| 25 | 23 | if err := ctx.Err(); err != nil { | |
| 26 | 24 | return err | |
| 27 | 25 | } | |
| @@ -31,9 +29,9 @@ func (r *Repository) Delete(ctx context.Context, name string) error { | |||
| 31 | 29 | // A prior rename may have succeeded while its parent sync failed. Complete | |
| 32 | 30 | // that durability boundary before considering a canonical directory: this | |
| 33 | 31 | // also leaves a newly recreated session untouched. | |
| 34 | - | pending, err := pendingQuarantine(sessions, key) | |
| 32 | + | pending, err := r.pendingQuarantine(sessions, key) | |
| 35 | 33 | if err != nil { | |
| 36 | - | return fmt.Errorf("read deleting snapshot session %q: %w", key, safeFilesystemError(err)) | |
| 34 | + | return fmt.Errorf("stat deleting snapshot session %q: %w", key, safeFilesystemError(err)) | |
| 37 | 35 | } | |
| 38 | 36 | if pending { | |
| 39 | 37 | if err := ctx.Err(); err != nil { | |
| @@ -49,15 +47,7 @@ func (r *Repository) Delete(ctx context.Context, name string) error { | |||
| 49 | 47 | } else if err != nil { | |
| 50 | 48 | return fmt.Errorf("stat snapshot session %q: %w", key, safeFilesystemError(err)) | |
| 51 | 49 | } | |
| 52 | - | quarantine := filepath.Join(sessions, ".deleting-"+key+"-"+fmt.Sprintf("%d", time.Now().UnixNano())) | |
| 53 | - | for attempt := 0; ; attempt++ { | |
| 54 | - | if _, err := os.Lstat(quarantine); errors.Is(err, os.ErrNotExist) { | |
| 55 | - | break | |
| 56 | - | } else if err != nil { | |
| 57 | - | return fmt.Errorf("stat deleting snapshot session %q: %w", key, safeFilesystemError(err)) | |
| 58 | - | } | |
| 59 | - | quarantine = filepath.Join(sessions, ".deleting-"+key+"-"+fmt.Sprintf("%d-%d", time.Now().UnixNano(), attempt)) | |
| 60 | - | } | |
| 50 | + | quarantine := filepath.Join(sessions, deletingSessionName(key)) | |
| 61 | 51 | if err := ctx.Err(); err != nil { | |
| 62 | 52 | return err | |
| 63 | 53 | } | |
| @@ -76,34 +66,25 @@ func (r *Repository) Delete(ctx context.Context, name string) error { | |||
| 76 | 66 | return nil | |
| 77 | 67 | } | |
| 78 | 68 | ||
| 79 | - | func pendingQuarantine(dir, key string) (pending bool, err error) { | |
| 80 | - | f, err := os.Open(dir) | |
| 69 | + | // deletingSessionName is a deterministic pending-delete record. Its presence | |
| 70 | + | // is checked directly rather than scanning an attacker-controlled sessions | |
| 71 | + | // directory, and it prevents a retry from deleting a session republished after | |
| 72 | + | // the initial rename reached disk but its directory sync failed. | |
| 73 | + | func deletingSessionName(key string) string { return ".deleting-" + key } | |
| 74 | + | ||
| 75 | + | func (r *Repository) pendingQuarantine(dir, key string) (bool, error) { | |
| 76 | + | path := filepath.Join(dir, deletingSessionName(key)) | |
| 77 | + | if hook := r.hooks.beforePendingQuarantineCheck; hook != nil { | |
| 78 | + | hook(path) | |
| 79 | + | } | |
| 80 | + | info, err := os.Lstat(path) | |
| 81 | 81 | if errors.Is(err, os.ErrNotExist) { | |
| 82 | 82 | return false, nil | |
| 83 | 83 | } | |
| 84 | 84 | if err != nil { | |
| 85 | 85 | return false, err | |
| 86 | 86 | } | |
| 87 | - | defer func() { | |
| 88 | - | if closeErr := f.Close(); closeErr != nil { | |
| 89 | - | err = errors.Join(err, fmt.Errorf("close deleting snapshot directory: %w", safeFilesystemError(closeErr))) | |
| 90 | - | } | |
| 91 | - | }() | |
| 92 | - | prefix := ".deleting-" + key + "-" | |
| 93 | - | for { | |
| 94 | - | entries, err := f.ReadDir(maintenanceBatch) | |
| 95 | - | for _, entry := range entries { | |
| 96 | - | if entry.IsDir() && strings.HasPrefix(entry.Name(), prefix) { | |
| 97 | - | return true, nil | |
| 98 | - | } | |
| 99 | - | } | |
| 100 | - | if errors.Is(err, io.EOF) { | |
| 101 | - | return false, nil | |
| 102 | - | } | |
| 103 | - | if err != nil { | |
| 104 | - | return false, err | |
| 105 | - | } | |
| 106 | - | } | |
| 87 | + | return info.IsDir(), nil | |
| 107 | 88 | } | |
| 108 | 89 | ||
| 109 | 90 | // Maintain reaps a bounded amount of stale state. Continuation handles are | |
| @@ -8,7 +8,6 @@ import ( | |||||
| 8 | "os" | 8 | "os" | ||
| 9 | "path/filepath" | 9 | "path/filepath" | ||
| 10 | "strings" | 10 | "strings" | ||
| 11 | - | "time" | |||
| 12 | ) | 11 | ) | ||
| 13 | 12 | ||||
| 14 | // Delete makes a session unavailable by durably moving it out of the canonical | 13 | // Delete makes a session unavailable by durably moving it out of the canonical | ||
| @@ -19,9 +18,8 @@ func (r *Repository) Delete(ctx context.Context, name string) error { | |||||
| 19 | return err | 18 | return err | ||
| 20 | } | 19 | } | ||
| 21 | key := sessionKey(name) | 20 | key := sessionKey(name) | ||
| 22 | - | lock := r.sessionLock(key) | 21 | + | lock := r.lockSession(key) |
| 23 | - | lock.Lock() | 22 | + | defer r.unlockSession(lock) |
| 24 | - | defer lock.Unlock() | |||
| 25 | if err := ctx.Err(); err != nil { | 23 | if err := ctx.Err(); err != nil { | ||
| 26 | return err | 24 | return err | ||
| 27 | } | 25 | } | ||
| @@ -31,9 +29,9 @@ func (r *Repository) Delete(ctx context.Context, name string) error { | |||||
| 31 | // A prior rename may have succeeded while its parent sync failed. Complete | 29 | // A prior rename may have succeeded while its parent sync failed. Complete | ||
| 32 | // that durability boundary before considering a canonical directory: this | 30 | // that durability boundary before considering a canonical directory: this | ||
| 33 | // also leaves a newly recreated session untouched. | 31 | // also leaves a newly recreated session untouched. | ||
| 34 | - | pending, err := pendingQuarantine(sessions, key) | 32 | + | pending, err := r.pendingQuarantine(sessions, key) |
| 35 | if err != nil { | 33 | if err != nil { | ||
| 36 | - | return fmt.Errorf("read deleting snapshot session %q: %w", key, safeFilesystemError(err)) | 34 | + | return fmt.Errorf("stat deleting snapshot session %q: %w", key, safeFilesystemError(err)) |
| 37 | } | 35 | } | ||
| 38 | if pending { | 36 | if pending { | ||
| 39 | if err := ctx.Err(); err != nil { | 37 | if err := ctx.Err(); err != nil { | ||
| @@ -49,15 +47,7 @@ func (r *Repository) Delete(ctx context.Context, name string) error { | |||||
| 49 | } else if err != nil { | 47 | } else if err != nil { | ||
| 50 | return fmt.Errorf("stat snapshot session %q: %w", key, safeFilesystemError(err)) | 48 | return fmt.Errorf("stat snapshot session %q: %w", key, safeFilesystemError(err)) | ||
| 51 | } | 49 | } | ||
| 52 | - | quarantine := filepath.Join(sessions, ".deleting-"+key+"-"+fmt.Sprintf("%d", time.Now().UnixNano())) | 50 | + | quarantine := filepath.Join(sessions, deletingSessionName(key)) |
| 53 | - | for attempt := 0; ; attempt++ { | |||
| 54 | - | if _, err := os.Lstat(quarantine); errors.Is(err, os.ErrNotExist) { | |||
| 55 | - | break | |||
| 56 | - | } else if err != nil { | |||
| 57 | - | return fmt.Errorf("stat deleting snapshot session %q: %w", key, safeFilesystemError(err)) | |||
| 58 | - | } | |||
| 59 | - | quarantine = filepath.Join(sessions, ".deleting-"+key+"-"+fmt.Sprintf("%d-%d", time.Now().UnixNano(), attempt)) | |||
| 60 | - | } | |||
| 61 | if err := ctx.Err(); err != nil { | 51 | if err := ctx.Err(); err != nil { | ||
| 62 | return err | 52 | return err | ||
| 63 | } | 53 | } | ||
| @@ -76,34 +66,25 @@ func (r *Repository) Delete(ctx context.Context, name string) error { | |||||
| 76 | return nil | 66 | return nil | ||
| 77 | } | 67 | } | ||
| 78 | 68 | ||||
| 79 | - | func pendingQuarantine(dir, key string) (pending bool, err error) { | 69 | + | // deletingSessionName is a deterministic pending-delete record. Its presence |
| 80 | - | f, err := os.Open(dir) | 70 | + | // is checked directly rather than scanning an attacker-controlled sessions |
| 71 | + | // directory, and it prevents a retry from deleting a session republished after | |||
| 72 | + | // the initial rename reached disk but its directory sync failed. | |||
| 73 | + | func deletingSessionName(key string) string { return ".deleting-" + key } | |||
| 74 | + | ||||
| 75 | + | func (r *Repository) pendingQuarantine(dir, key string) (bool, error) { | |||
| 76 | + | path := filepath.Join(dir, deletingSessionName(key)) | |||
| 77 | + | if hook := r.hooks.beforePendingQuarantineCheck; hook != nil { | |||
| 78 | + | hook(path) | |||
| 79 | + | } | |||
| 80 | + | info, err := os.Lstat(path) | |||
| 81 | if errors.Is(err, os.ErrNotExist) { | 81 | if errors.Is(err, os.ErrNotExist) { | ||
| 82 | return false, nil | 82 | return false, nil | ||
| 83 | } | 83 | } | ||
| 84 | if err != nil { | 84 | if err != nil { | ||
| 85 | return false, err | 85 | return false, err | ||
| 86 | } | 86 | } | ||
| 87 | - | defer func() { | 87 | + | return info.IsDir(), nil |
| 88 | - | if closeErr := f.Close(); closeErr != nil { | |||
| 89 | - | err = errors.Join(err, fmt.Errorf("close deleting snapshot directory: %w", safeFilesystemError(closeErr))) | |||
| 90 | - | } | |||
| 91 | - | }() | |||
| 92 | - | prefix := ".deleting-" + key + "-" | |||
| 93 | - | for { | |||
| 94 | - | entries, err := f.ReadDir(maintenanceBatch) | |||
| 95 | - | for _, entry := range entries { | |||
| 96 | - | if entry.IsDir() && strings.HasPrefix(entry.Name(), prefix) { | |||
| 97 | - | return true, nil | |||
| 98 | - | } | |||
| 99 | - | } | |||
| 100 | - | if errors.Is(err, io.EOF) { | |||
| 101 | - | return false, nil | |||
| 102 | - | } | |||
| 103 | - | if err != nil { | |||
| 104 | - | return false, err | |||
| 105 | - | } | |||
| 106 | - | } | |||
| 107 | } | 88 | } | ||
| 108 | 89 | ||||
| 109 | // Maintain reaps a bounded amount of stale state. Continuation handles are | 90 | // Maintain reaps a bounded amount of stale state. Continuation handles are | ||
M internal/adapters/snapshot/maintenance_gc.go +5 -0
| @@ -19,6 +19,7 @@ type manifestMaintenance struct { | |||
| 19 | 19 | } | |
| 20 | 20 | ||
| 21 | 21 | type sessionMaintenance struct { | |
| 22 | + | lock *sessionMutex | |
| 22 | 23 | token string | |
| 23 | 24 | epoch uint64 | |
| 24 | 25 | marked map[uint64]manifestMaintenance | |
| @@ -115,6 +116,7 @@ func (r *Repository) sessionMaintenanceState(key string) (*sessionMaintenance, e | |||
| 115 | 116 | if state == nil || state.token != token || state.epoch != r.storageEpoch(key) { | |
| 116 | 117 | r.clearSessionMaintenance(key) | |
| 117 | 118 | state = &sessionMaintenance{ | |
| 119 | + | lock: r.retainSessionState(key), | |
| 118 | 120 | token: token, | |
| 119 | 121 | epoch: r.storageEpoch(key), | |
| 120 | 122 | conservative: conservative, | |
| @@ -144,6 +146,9 @@ func (r *Repository) maintenanceToken(key string) (string, bool, error) { | |||
| 144 | 146 | } | |
| 145 | 147 | ||
| 146 | 148 | func (r *Repository) clearSessionMaintenance(key string) { | |
| 149 | + | if state := r.maintenanceSessions[key]; state != nil && state.lock != nil { | |
| 150 | + | r.releaseSessionReference(state.lock) | |
| 151 | + | } | |
| 147 | 152 | delete(r.maintenanceSessions, key) | |
| 148 | 153 | prefix := "\x00" + filepath.Clean(r.sessionPath(key)) | |
| 149 | 154 | for id := range r.maintenanceCursors { | |
| @@ -19,6 +19,7 @@ type manifestMaintenance struct { | |||||
| 19 | } | 19 | } | ||
| 20 | 20 | ||||
| 21 | type sessionMaintenance struct { | 21 | type sessionMaintenance struct { | ||
| 22 | + | lock *sessionMutex | |||
| 22 | token string | 23 | token string | ||
| 23 | epoch uint64 | 24 | epoch uint64 | ||
| 24 | marked map[uint64]manifestMaintenance | 25 | marked map[uint64]manifestMaintenance | ||
| @@ -115,6 +116,7 @@ func (r *Repository) sessionMaintenanceState(key string) (*sessionMaintenance, e | |||||
| 115 | if state == nil || state.token != token || state.epoch != r.storageEpoch(key) { | 116 | if state == nil || state.token != token || state.epoch != r.storageEpoch(key) { | ||
| 116 | r.clearSessionMaintenance(key) | 117 | r.clearSessionMaintenance(key) | ||
| 117 | state = &sessionMaintenance{ | 118 | state = &sessionMaintenance{ | ||
| 119 | + | lock: r.retainSessionState(key), | |||
| 118 | token: token, | 120 | token: token, | ||
| 119 | epoch: r.storageEpoch(key), | 121 | epoch: r.storageEpoch(key), | ||
| 120 | conservative: conservative, | 122 | conservative: conservative, | ||
| @@ -144,6 +146,9 @@ func (r *Repository) maintenanceToken(key string) (string, bool, error) { | |||||
| 144 | } | 146 | } | ||
| 145 | 147 | ||||
| 146 | func (r *Repository) clearSessionMaintenance(key string) { | 148 | func (r *Repository) clearSessionMaintenance(key string) { | ||
| 149 | + | if state := r.maintenanceSessions[key]; state != nil && state.lock != nil { | |||
| 150 | + | r.releaseSessionReference(state.lock) | |||
| 151 | + | } | |||
| 147 | delete(r.maintenanceSessions, key) | 152 | delete(r.maintenanceSessions, key) | ||
| 148 | prefix := "\x00" + filepath.Clean(r.sessionPath(key)) | 153 | prefix := "\x00" + filepath.Clean(r.sessionPath(key)) | ||
| 149 | for id := range r.maintenanceCursors { | 154 | for id := range r.maintenanceCursors { | ||
M internal/adapters/snapshot/maintenance_test.go +6 -9
| @@ -224,8 +224,7 @@ func TestRepositoryCancellationAfterSessionLockWaitPreventsMutations(t *testing. | |||
| 224 | 224 | t.Run("publish", func(t *testing.T) { | |
| 225 | 225 | repo := NewRepository(privateDir(t)) | |
| 226 | 226 | key := sessionKey("named") | |
| 227 | - | lock := repo.sessionLock(key) | |
| 228 | - | lock.Lock() | |
| 227 | + | lock := repo.lockSession(key) | |
| 229 | 228 | ctx, cancel := context.WithCancel(context.Background()) | |
| 230 | 229 | reached := make(chan struct{}) | |
| 231 | 230 | release := make(chan struct{}) | |
| @@ -235,7 +234,7 @@ func TestRepositoryCancellationAfterSessionLockWaitPreventsMutations(t *testing. | |||
| 235 | 234 | <-reached | |
| 236 | 235 | cancel() | |
| 237 | 236 | close(release) | |
| 238 | - | lock.Unlock() | |
| 237 | + | repo.unlockSession(lock) | |
| 239 | 238 | if err := <-done; !errors.Is(err, context.Canceled) { | |
| 240 | 239 | t.Fatalf("Publish error = %v, want canceled", err) | |
| 241 | 240 | } | |
| @@ -257,8 +256,7 @@ func TestRepositoryCancellationAfterSessionLockWaitPreventsMutations(t *testing. | |||
| 257 | 256 | {"delete", func(ctx context.Context) error { return repo.Delete(ctx, "named") }}, | |
| 258 | 257 | } { | |
| 259 | 258 | t.Run(operation.name, func(t *testing.T) { | |
| 260 | - | lock := repo.sessionLock(key) | |
| 261 | - | lock.Lock() | |
| 259 | + | lock := repo.lockSession(key) | |
| 262 | 260 | ctx, cancel := context.WithCancel(context.Background()) | |
| 263 | 261 | reached := make(chan struct{}) | |
| 264 | 262 | release := make(chan struct{}) | |
| @@ -268,7 +266,7 @@ func TestRepositoryCancellationAfterSessionLockWaitPreventsMutations(t *testing. | |||
| 268 | 266 | <-reached | |
| 269 | 267 | cancel() | |
| 270 | 268 | close(release) | |
| 271 | - | lock.Unlock() | |
| 269 | + | repo.unlockSession(lock) | |
| 272 | 270 | if err := <-done; !errors.Is(err, context.Canceled) { | |
| 273 | 271 | t.Fatalf("%s error = %v, want canceled", operation.name, err) | |
| 274 | 272 | } | |
| @@ -284,8 +282,7 @@ func TestRepositoryCancellationAfterSessionLockWaitPreventsMutations(t *testing. | |||
| 284 | 282 | if err := repo.Publish(context.Background(), repositoryPublication(t, "named", 1, []byte("state"))); err != nil { | |
| 285 | 283 | t.Fatal(err) | |
| 286 | 284 | } | |
| 287 | - | lock := repo.sessionLock(sessionKey("named")) | |
| 288 | - | lock.Lock() | |
| 285 | + | lock := repo.lockSession(sessionKey("named")) | |
| 289 | 286 | ctx, cancel := context.WithCancel(context.Background()) | |
| 290 | 287 | reached := make(chan struct{}) | |
| 291 | 288 | release := make(chan struct{}) | |
| @@ -295,7 +292,7 @@ func TestRepositoryCancellationAfterSessionLockWaitPreventsMutations(t *testing. | |||
| 295 | 292 | <-reached | |
| 296 | 293 | cancel() | |
| 297 | 294 | close(release) | |
| 298 | - | lock.Unlock() | |
| 295 | + | repo.unlockSession(lock) | |
| 299 | 296 | if err := <-done; !errors.Is(err, context.Canceled) { | |
| 300 | 297 | t.Fatalf("Maintain error = %v, want canceled", err) | |
| 301 | 298 | } | |
| @@ -224,8 +224,7 @@ func TestRepositoryCancellationAfterSessionLockWaitPreventsMutations(t *testing. | |||||
| 224 | t.Run("publish", func(t *testing.T) { | 224 | t.Run("publish", func(t *testing.T) { | ||
| 225 | repo := NewRepository(privateDir(t)) | 225 | repo := NewRepository(privateDir(t)) | ||
| 226 | key := sessionKey("named") | 226 | key := sessionKey("named") | ||
| 227 | - | lock := repo.sessionLock(key) | 227 | + | lock := repo.lockSession(key) |
| 228 | - | lock.Lock() | |||
| 229 | ctx, cancel := context.WithCancel(context.Background()) | 228 | ctx, cancel := context.WithCancel(context.Background()) | ||
| 230 | reached := make(chan struct{}) | 229 | reached := make(chan struct{}) | ||
| 231 | release := make(chan struct{}) | 230 | release := make(chan struct{}) | ||
| @@ -235,7 +234,7 @@ func TestRepositoryCancellationAfterSessionLockWaitPreventsMutations(t *testing. | |||||
| 235 | <-reached | 234 | <-reached | ||
| 236 | cancel() | 235 | cancel() | ||
| 237 | close(release) | 236 | close(release) | ||
| 238 | - | lock.Unlock() | 237 | + | repo.unlockSession(lock) |
| 239 | if err := <-done; !errors.Is(err, context.Canceled) { | 238 | if err := <-done; !errors.Is(err, context.Canceled) { | ||
| 240 | t.Fatalf("Publish error = %v, want canceled", err) | 239 | t.Fatalf("Publish error = %v, want canceled", err) | ||
| 241 | } | 240 | } | ||
| @@ -257,8 +256,7 @@ func TestRepositoryCancellationAfterSessionLockWaitPreventsMutations(t *testing. | |||||
| 257 | {"delete", func(ctx context.Context) error { return repo.Delete(ctx, "named") }}, | 256 | {"delete", func(ctx context.Context) error { return repo.Delete(ctx, "named") }}, | ||
| 258 | } { | 257 | } { | ||
| 259 | t.Run(operation.name, func(t *testing.T) { | 258 | t.Run(operation.name, func(t *testing.T) { | ||
| 260 | - | lock := repo.sessionLock(key) | 259 | + | lock := repo.lockSession(key) |
| 261 | - | lock.Lock() | |||
| 262 | ctx, cancel := context.WithCancel(context.Background()) | 260 | ctx, cancel := context.WithCancel(context.Background()) | ||
| 263 | reached := make(chan struct{}) | 261 | reached := make(chan struct{}) | ||
| 264 | release := make(chan struct{}) | 262 | release := make(chan struct{}) | ||
| @@ -268,7 +266,7 @@ func TestRepositoryCancellationAfterSessionLockWaitPreventsMutations(t *testing. | |||||
| 268 | <-reached | 266 | <-reached | ||
| 269 | cancel() | 267 | cancel() | ||
| 270 | close(release) | 268 | close(release) | ||
| 271 | - | lock.Unlock() | 269 | + | repo.unlockSession(lock) |
| 272 | if err := <-done; !errors.Is(err, context.Canceled) { | 270 | if err := <-done; !errors.Is(err, context.Canceled) { | ||
| 273 | t.Fatalf("%s error = %v, want canceled", operation.name, err) | 271 | t.Fatalf("%s error = %v, want canceled", operation.name, err) | ||
| 274 | } | 272 | } | ||
| @@ -284,8 +282,7 @@ func TestRepositoryCancellationAfterSessionLockWaitPreventsMutations(t *testing. | |||||
| 284 | if err := repo.Publish(context.Background(), repositoryPublication(t, "named", 1, []byte("state"))); err != nil { | 282 | if err := repo.Publish(context.Background(), repositoryPublication(t, "named", 1, []byte("state"))); err != nil { | ||
| 285 | t.Fatal(err) | 283 | t.Fatal(err) | ||
| 286 | } | 284 | } | ||
| 287 | - | lock := repo.sessionLock(sessionKey("named")) | 285 | + | lock := repo.lockSession(sessionKey("named")) |
| 288 | - | lock.Lock() | |||
| 289 | ctx, cancel := context.WithCancel(context.Background()) | 286 | ctx, cancel := context.WithCancel(context.Background()) | ||
| 290 | reached := make(chan struct{}) | 287 | reached := make(chan struct{}) | ||
| 291 | release := make(chan struct{}) | 288 | release := make(chan struct{}) | ||
| @@ -295,7 +292,7 @@ func TestRepositoryCancellationAfterSessionLockWaitPreventsMutations(t *testing. | |||||
| 295 | <-reached | 292 | <-reached | ||
| 296 | cancel() | 293 | cancel() | ||
| 297 | close(release) | 294 | close(release) | ||
| 298 | - | lock.Unlock() | 295 | + | repo.unlockSession(lock) |
| 299 | if err := <-done; !errors.Is(err, context.Canceled) { | 296 | if err := <-done; !errors.Is(err, context.Canceled) { | ||
| 300 | t.Fatalf("Maintain error = %v, want canceled", err) | 297 | t.Fatalf("Maintain error = %v, want canceled", err) | ||
| 301 | } | 298 | } | ||
M internal/adapters/snapshot/publish.go +2 -3
| @@ -14,9 +14,8 @@ func (r *Repository) Publish(ctx context.Context, publication ports.SnapshotPubl | |||
| 14 | 14 | return err | |
| 15 | 15 | } | |
| 16 | 16 | key := sessionKey(publication.Name) | |
| 17 | - | lock := r.sessionLock(key) | |
| 18 | - | lock.Lock() | |
| 19 | - | defer lock.Unlock() | |
| 17 | + | lock := r.lockSession(key) | |
| 18 | + | defer r.unlockSession(lock) | |
| 20 | 19 | if err := ctx.Err(); err != nil { | |
| 21 | 20 | return err | |
| 22 | 21 | } | |
| @@ -14,9 +14,8 @@ func (r *Repository) Publish(ctx context.Context, publication ports.SnapshotPubl | |||||
| 14 | return err | 14 | return err | ||
| 15 | } | 15 | } | ||
| 16 | key := sessionKey(publication.Name) | 16 | key := sessionKey(publication.Name) | ||
| 17 | - | lock := r.sessionLock(key) | 17 | + | lock := r.lockSession(key) |
| 18 | - | lock.Lock() | 18 | + | defer r.unlockSession(lock) |
| 19 | - | defer lock.Unlock() | |||
| 20 | if err := ctx.Err(); err != nil { | 19 | if err := ctx.Err(); err != nil { | ||
| 21 | return err | 20 | return err | ||
| 22 | } | 21 | } | ||
M internal/adapters/snapshot/repository.go +97 -36
| @@ -23,13 +23,15 @@ const ( | |||
| 23 | 23 | // Store remains available separately for the one-way legacy bridge. | |
| 24 | 24 | type Repository struct { | |
| 25 | 25 | dir string | |
| 26 | - | locks sync.Map // map[string]*sessionMutex | |
| 27 | 26 | hooks repositoryHooks | |
| 28 | 27 | ||
| 29 | - | // storageEpochs is guarded by each session's lock. It invalidates a | |
| 30 | - | // maintenance mark whenever publication or deletion changes that session's | |
| 31 | - | // on-disk namespace. | |
| 32 | - | storageEpochs map[string]uint64 | |
| 28 | + | // sessionStateMu owns both maps. A caller retains a reference before it | |
| 29 | + | // waits on a session mutex, so an idle entry can never be removed while a | |
| 30 | + | // waiter still holds its old mutex. Epoch entries share that lifetime. | |
| 31 | + | sessionStateMu sync.Mutex | |
| 32 | + | locks map[string]*sessionMutex | |
| 33 | + | storageEpochs map[string]uint64 | |
| 34 | + | nextEpoch uint64 | |
| 33 | 35 | ||
| 34 | 36 | // maintenanceMu owns bounded continuation metadata: seek cookies and | |
| 35 | 37 | // pending directory entries. Directory descriptors are opened and closed | |
| @@ -53,21 +55,22 @@ type repositoryHooks struct { | |||
| 53 | 55 | // Object hooks instrument the publication path in package tests. They keep | |
| 54 | 56 | // the steady-state cost of retained history observable without exposing a | |
| 55 | 57 | // production metrics surface. | |
| 56 | - | beforeObjectRead func(string) | |
| 57 | - | beforeObjectHash func([]byte) | |
| 58 | - | beforeObjectCopy func([]byte) | |
| 59 | - | beforeHeadWrite func(string) error | |
| 60 | - | createTemp func(string) error | |
| 61 | - | writeTemp func(string) error | |
| 62 | - | syncFile func(string) error | |
| 63 | - | closeFile func(string) error | |
| 64 | - | installImmutable func(string) error | |
| 65 | - | rename func(string) error | |
| 66 | - | syncDirectory func(string) error | |
| 67 | - | remove func(string) error | |
| 68 | - | openMaintenanceDirectory func(string) (maintenanceDirectory, error) | |
| 69 | - | openLegacyDirectory func(string) (legacyDirectory, error) | |
| 70 | - | beforeSessionLock func(string) | |
| 58 | + | beforeObjectRead func(string) | |
| 59 | + | beforeObjectHash func([]byte) | |
| 60 | + | beforeObjectCopy func([]byte) | |
| 61 | + | beforeHeadWrite func(string) error | |
| 62 | + | beforePendingQuarantineCheck func(string) | |
| 63 | + | createTemp func(string) error | |
| 64 | + | writeTemp func(string) error | |
| 65 | + | syncFile func(string) error | |
| 66 | + | closeFile func(string) error | |
| 67 | + | installImmutable func(string) error | |
| 68 | + | rename func(string) error | |
| 69 | + | syncDirectory func(string) error | |
| 70 | + | remove func(string) error | |
| 71 | + | openMaintenanceDirectory func(string) (maintenanceDirectory, error) | |
| 72 | + | openLegacyDirectory func(string) (legacyDirectory, error) | |
| 73 | + | beforeSessionLock func(string) | |
| 71 | 74 | } | |
| 72 | 75 | ||
| 73 | 76 | var _ ports.SnapshotRepository = (*Repository)(nil) | |
| @@ -76,35 +79,93 @@ var _ ports.LegacySnapshotSource = (*Repository)(nil) | |||
| 76 | 79 | // NewRepository creates a repository rooted at dir. It does not create files | |
| 77 | 80 | // until the first publication, so merely constructing it is side-effect free. | |
| 78 | 81 | func NewRepository(dir string) *Repository { | |
| 79 | - | return &Repository{dir: dir, storageEpochs: make(map[string]uint64)} | |
| 82 | + | return &Repository{ | |
| 83 | + | dir: dir, | |
| 84 | + | locks: make(map[string]*sessionMutex), | |
| 85 | + | storageEpochs: make(map[string]uint64), | |
| 86 | + | } | |
| 80 | 87 | } | |
| 81 | 88 | ||
| 82 | - | // invalidateStorageEpoch must be called with sessionLock(key) held. | |
| 89 | + | // invalidateStorageEpoch must be called with lockSession(key) held. The | |
| 90 | + | // registry mutex makes accesses for different session keys race-free too. | |
| 83 | 91 | func (r *Repository) invalidateStorageEpoch(key string) { | |
| 84 | - | r.storageEpochs[key]++ | |
| 92 | + | r.sessionStateMu.Lock() | |
| 93 | + | r.nextEpoch++ | |
| 94 | + | r.storageEpochs[key] = r.nextEpoch | |
| 95 | + | r.sessionStateMu.Unlock() | |
| 85 | 96 | } | |
| 86 | 97 | ||
| 87 | - | // storageEpoch must be called with sessionLock(key) held. | |
| 88 | - | func (r *Repository) storageEpoch(key string) uint64 { return r.storageEpochs[key] } | |
| 98 | + | // storageEpoch must be called with lockSession(key) held. | |
| 99 | + | func (r *Repository) storageEpoch(key string) uint64 { | |
| 100 | + | r.sessionStateMu.Lock() | |
| 101 | + | epoch := r.storageEpochs[key] | |
| 102 | + | r.sessionStateMu.Unlock() | |
| 103 | + | return epoch | |
| 104 | + | } | |
| 89 | 105 | ||
| 90 | - | // sessionMutex provides a test-only boundary immediately before a contested | |
| 91 | - | // session mutex. Production hooks are nil, so normal locking is unchanged. | |
| 106 | + | // sessionMutex is retained by each waiter and each resumable maintenance | |
| 107 | + | // state. That reference is what makes eviction safe: a new caller cannot lock | |
| 108 | + | // a replacement mutex until all users of the prior mutex have left it. | |
| 92 | 109 | type sessionMutex struct { | |
| 93 | - | repository *Repository | |
| 94 | 110 | key string | |
| 95 | 111 | mu sync.Mutex | |
| 112 | + | references int | |
| 96 | 113 | } | |
| 97 | 114 | ||
| 98 | - | func (m *sessionMutex) Lock() { | |
| 99 | - | if hook := m.repository.hooks.beforeSessionLock; hook != nil { | |
| 100 | - | hook(m.key) | |
| 115 | + | func (r *Repository) lockSession(key string) *sessionMutex { | |
| 116 | + | r.sessionStateMu.Lock() | |
| 117 | + | lock := r.locks[key] | |
| 118 | + | if lock == nil { | |
| 119 | + | lock = &sessionMutex{key: key} | |
| 120 | + | r.locks[key] = lock | |
| 121 | + | } | |
| 122 | + | lock.references++ | |
| 123 | + | r.sessionStateMu.Unlock() | |
| 124 | + | if hook := r.hooks.beforeSessionLock; hook != nil { | |
| 125 | + | hook(key) | |
| 101 | 126 | } | |
| 102 | - | m.mu.Lock() | |
| 127 | + | lock.mu.Lock() | |
| 128 | + | return lock | |
| 103 | 129 | } | |
| 104 | 130 | ||
| 105 | - | func (m *sessionMutex) Unlock() { m.mu.Unlock() } | |
| 131 | + | func (r *Repository) unlockSession(lock *sessionMutex) { | |
| 132 | + | lock.mu.Unlock() | |
| 133 | + | r.releaseSessionReference(lock) | |
| 134 | + | } | |
| 135 | + | ||
| 136 | + | func (r *Repository) retainSessionState(key string) *sessionMutex { | |
| 137 | + | r.sessionStateMu.Lock() | |
| 138 | + | lock := r.locks[key] | |
| 139 | + | // sessionMaintenanceState is called while the active session lock is held. | |
| 140 | + | if lock == nil { | |
| 141 | + | panic("snapshot: retain missing session lock") | |
| 142 | + | } | |
| 143 | + | lock.references++ | |
| 144 | + | r.sessionStateMu.Unlock() | |
| 145 | + | return lock | |
| 146 | + | } | |
| 147 | + | ||
| 148 | + | func (r *Repository) releaseSessionReference(lock *sessionMutex) { | |
| 149 | + | r.sessionStateMu.Lock() | |
| 150 | + | lock.references-- | |
| 151 | + | if lock.references < 0 { | |
| 152 | + | r.sessionStateMu.Unlock() | |
| 153 | + | panic("snapshot: release of unretained session lock") | |
| 154 | + | } | |
| 155 | + | if lock.references == 0 { | |
| 156 | + | if r.locks[lock.key] != lock { | |
| 157 | + | r.sessionStateMu.Unlock() | |
| 158 | + | panic("snapshot: session lock registry mismatch") | |
| 159 | + | } | |
| 160 | + | delete(r.locks, lock.key) | |
| 161 | + | delete(r.storageEpochs, lock.key) | |
| 162 | + | } | |
| 163 | + | r.sessionStateMu.Unlock() | |
| 164 | + | } | |
| 106 | 165 | ||
| 107 | - | func (r *Repository) sessionLock(key string) *sessionMutex { | |
| 108 | - | lock, _ := r.locks.LoadOrStore(key, &sessionMutex{repository: r, key: key}) | |
| 109 | - | return lock.(*sessionMutex) | |
| 166 | + | // sessionStateCounts is test evidence that idle per-session state is evicted. | |
| 167 | + | func (r *Repository) sessionStateCounts() (locks, epochs int) { | |
| 168 | + | r.sessionStateMu.Lock() | |
| 169 | + | defer r.sessionStateMu.Unlock() | |
| 170 | + | return len(r.locks), len(r.storageEpochs) | |
| 110 | 171 | } | |
| @@ -23,13 +23,15 @@ const ( | |||||
| 23 | // Store remains available separately for the one-way legacy bridge. | 23 | // Store remains available separately for the one-way legacy bridge. | ||
| 24 | type Repository struct { | 24 | type Repository struct { | ||
| 25 | dir string | 25 | dir string | ||
| 26 | - | locks sync.Map // map[string]*sessionMutex | |||
| 27 | hooks repositoryHooks | 26 | hooks repositoryHooks | ||
| 28 | 27 | ||||
| 29 | - | // storageEpochs is guarded by each session's lock. It invalidates a | 28 | + | // sessionStateMu owns both maps. A caller retains a reference before it |
| 30 | - | // maintenance mark whenever publication or deletion changes that session's | 29 | + | // waits on a session mutex, so an idle entry can never be removed while a |
| 31 | - | // on-disk namespace. | 30 | + | // waiter still holds its old mutex. Epoch entries share that lifetime. |
| 32 | - | storageEpochs map[string]uint64 | 31 | + | sessionStateMu sync.Mutex |
| 32 | + | locks map[string]*sessionMutex | |||
| 33 | + | storageEpochs map[string]uint64 | |||
| 34 | + | nextEpoch uint64 | |||
| 33 | 35 | ||||
| 34 | // maintenanceMu owns bounded continuation metadata: seek cookies and | 36 | // maintenanceMu owns bounded continuation metadata: seek cookies and | ||
| 35 | // pending directory entries. Directory descriptors are opened and closed | 37 | // pending directory entries. Directory descriptors are opened and closed | ||
| @@ -53,21 +55,22 @@ type repositoryHooks struct { | |||||
| 53 | // Object hooks instrument the publication path in package tests. They keep | 55 | // Object hooks instrument the publication path in package tests. They keep | ||
| 54 | // the steady-state cost of retained history observable without exposing a | 56 | // the steady-state cost of retained history observable without exposing a | ||
| 55 | // production metrics surface. | 57 | // production metrics surface. | ||
| 56 | - | beforeObjectRead func(string) | 58 | + | beforeObjectRead func(string) |
| 57 | - | beforeObjectHash func([]byte) | 59 | + | beforeObjectHash func([]byte) |
| 58 | - | beforeObjectCopy func([]byte) | 60 | + | beforeObjectCopy func([]byte) |
| 59 | - | beforeHeadWrite func(string) error | 61 | + | beforeHeadWrite func(string) error |
| 60 | - | createTemp func(string) error | 62 | + | beforePendingQuarantineCheck func(string) |
| 61 | - | writeTemp func(string) error | 63 | + | createTemp func(string) error |
| 62 | - | syncFile func(string) error | 64 | + | writeTemp func(string) error |
| 63 | - | closeFile func(string) error | 65 | + | syncFile func(string) error |
| 64 | - | installImmutable func(string) error | 66 | + | closeFile func(string) error |
| 65 | - | rename func(string) error | 67 | + | installImmutable func(string) error |
| 66 | - | syncDirectory func(string) error | 68 | + | rename func(string) error |
| 67 | - | remove func(string) error | 69 | + | syncDirectory func(string) error |
| 68 | - | openMaintenanceDirectory func(string) (maintenanceDirectory, error) | 70 | + | remove func(string) error |
| 69 | - | openLegacyDirectory func(string) (legacyDirectory, error) | 71 | + | openMaintenanceDirectory func(string) (maintenanceDirectory, error) |
| 70 | - | beforeSessionLock func(string) | 72 | + | openLegacyDirectory func(string) (legacyDirectory, error) |
| 73 | + | beforeSessionLock func(string) | |||
| 71 | } | 74 | } | ||
| 72 | 75 | ||||
| 73 | var _ ports.SnapshotRepository = (*Repository)(nil) | 76 | var _ ports.SnapshotRepository = (*Repository)(nil) | ||
| @@ -76,35 +79,93 @@ var _ ports.LegacySnapshotSource = (*Repository)(nil) | |||||
| 76 | // NewRepository creates a repository rooted at dir. It does not create files | 79 | // NewRepository creates a repository rooted at dir. It does not create files | ||
| 77 | // until the first publication, so merely constructing it is side-effect free. | 80 | // until the first publication, so merely constructing it is side-effect free. | ||
| 78 | func NewRepository(dir string) *Repository { | 81 | func NewRepository(dir string) *Repository { | ||
| 79 | - | return &Repository{dir: dir, storageEpochs: make(map[string]uint64)} | 82 | + | return &Repository{ |
| 83 | + | dir: dir, | |||
| 84 | + | locks: make(map[string]*sessionMutex), | |||
| 85 | + | storageEpochs: make(map[string]uint64), | |||
| 86 | + | } | |||
| 80 | } | 87 | } | ||
| 81 | 88 | ||||
| 82 | - | // invalidateStorageEpoch must be called with sessionLock(key) held. | 89 | + | // invalidateStorageEpoch must be called with lockSession(key) held. The |
| 90 | + | // registry mutex makes accesses for different session keys race-free too. | |||
| 83 | func (r *Repository) invalidateStorageEpoch(key string) { | 91 | func (r *Repository) invalidateStorageEpoch(key string) { | ||
| 84 | - | r.storageEpochs[key]++ | 92 | + | r.sessionStateMu.Lock() |
| 93 | + | r.nextEpoch++ | |||
| 94 | + | r.storageEpochs[key] = r.nextEpoch | |||
| 95 | + | r.sessionStateMu.Unlock() | |||
| 85 | } | 96 | } | ||
| 86 | 97 | ||||
| 87 | - | // storageEpoch must be called with sessionLock(key) held. | 98 | + | // storageEpoch must be called with lockSession(key) held. |
| 88 | - | func (r *Repository) storageEpoch(key string) uint64 { return r.storageEpochs[key] } | 99 | + | func (r *Repository) storageEpoch(key string) uint64 { |
| 100 | + | r.sessionStateMu.Lock() | |||
| 101 | + | epoch := r.storageEpochs[key] | |||
| 102 | + | r.sessionStateMu.Unlock() | |||
| 103 | + | return epoch | |||
| 104 | + | } | |||
| 89 | 105 | ||||
| 90 | - | // sessionMutex provides a test-only boundary immediately before a contested | 106 | + | // sessionMutex is retained by each waiter and each resumable maintenance |
| 91 | - | // session mutex. Production hooks are nil, so normal locking is unchanged. | 107 | + | // state. That reference is what makes eviction safe: a new caller cannot lock |
| 108 | + | // a replacement mutex until all users of the prior mutex have left it. | |||
| 92 | type sessionMutex struct { | 109 | type sessionMutex struct { | ||
| 93 | - | repository *Repository | |||
| 94 | key string | 110 | key string | ||
| 95 | mu sync.Mutex | 111 | mu sync.Mutex | ||
| 112 | + | references int | |||
| 96 | } | 113 | } | ||
| 97 | 114 | ||||
| 98 | - | func (m *sessionMutex) Lock() { | 115 | + | func (r *Repository) lockSession(key string) *sessionMutex { |
| 99 | - | if hook := m.repository.hooks.beforeSessionLock; hook != nil { | 116 | + | r.sessionStateMu.Lock() |
| 100 | - | hook(m.key) | 117 | + | lock := r.locks[key] |
| 118 | + | if lock == nil { | |||
| 119 | + | lock = &sessionMutex{key: key} | |||
| 120 | + | r.locks[key] = lock | |||
| 121 | + | } | |||
| 122 | + | lock.references++ | |||
| 123 | + | r.sessionStateMu.Unlock() | |||
| 124 | + | if hook := r.hooks.beforeSessionLock; hook != nil { | |||
| 125 | + | hook(key) | |||
| 101 | } | 126 | } | ||
| 102 | - | m.mu.Lock() | 127 | + | lock.mu.Lock() |
| 128 | + | return lock | |||
| 103 | } | 129 | } | ||
| 104 | 130 | ||||
| 105 | - | func (m *sessionMutex) Unlock() { m.mu.Unlock() } | 131 | + | func (r *Repository) unlockSession(lock *sessionMutex) { |
| 132 | + | lock.mu.Unlock() | |||
| 133 | + | r.releaseSessionReference(lock) | |||
| 134 | + | } | |||
| 135 | + | ||||
| 136 | + | func (r *Repository) retainSessionState(key string) *sessionMutex { | |||
| 137 | + | r.sessionStateMu.Lock() | |||
| 138 | + | lock := r.locks[key] | |||
| 139 | + | // sessionMaintenanceState is called while the active session lock is held. | |||
| 140 | + | if lock == nil { | |||
| 141 | + | panic("snapshot: retain missing session lock") | |||
| 142 | + | } | |||
| 143 | + | lock.references++ | |||
| 144 | + | r.sessionStateMu.Unlock() | |||
| 145 | + | return lock | |||
| 146 | + | } | |||
| 147 | + | ||||
| 148 | + | func (r *Repository) releaseSessionReference(lock *sessionMutex) { | |||
| 149 | + | r.sessionStateMu.Lock() | |||
| 150 | + | lock.references-- | |||
| 151 | + | if lock.references < 0 { | |||
| 152 | + | r.sessionStateMu.Unlock() | |||
| 153 | + | panic("snapshot: release of unretained session lock") | |||
| 154 | + | } | |||
| 155 | + | if lock.references == 0 { | |||
| 156 | + | if r.locks[lock.key] != lock { | |||
| 157 | + | r.sessionStateMu.Unlock() | |||
| 158 | + | panic("snapshot: session lock registry mismatch") | |||
| 159 | + | } | |||
| 160 | + | delete(r.locks, lock.key) | |||
| 161 | + | delete(r.storageEpochs, lock.key) | |||
| 162 | + | } | |||
| 163 | + | r.sessionStateMu.Unlock() | |||
| 164 | + | } | |||
| 106 | 165 | ||||
| 107 | - | func (r *Repository) sessionLock(key string) *sessionMutex { | 166 | + | // sessionStateCounts is test evidence that idle per-session state is evicted. |
| 108 | - | lock, _ := r.locks.LoadOrStore(key, &sessionMutex{repository: r, key: key}) | 167 | + | func (r *Repository) sessionStateCounts() (locks, epochs int) { |
| 109 | - | return lock.(*sessionMutex) | 168 | + | r.sessionStateMu.Lock() |
| 169 | + | defer r.sessionStateMu.Unlock() | |||
| 170 | + | return len(r.locks), len(r.storageEpochs) | |||
| 110 | } | 171 | } | ||
A internal/adapters/snapshot/repository_phase6_test.go +107 -0
| @@ -0,0 +1,107 @@ | |||
| 1 | + | package snapshot | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | "fmt" | |
| 6 | + | "os" | |
| 7 | + | "path/filepath" | |
| 8 | + | "sync" | |
| 9 | + | "sync/atomic" | |
| 10 | + | "testing" | |
| 11 | + | ||
| 12 | + | "github.com/bnema/vev/internal/ports" | |
| 13 | + | ) | |
| 14 | + | ||
| 15 | + | func TestRepositoryPublishDeleteManySessionsRaceFree(t *testing.T) { | |
| 16 | + | repo := NewRepository(privateDir(t)) | |
| 17 | + | const sessions = 128 | |
| 18 | + | ||
| 19 | + | publications := make([]ports.SnapshotPublication, sessions) | |
| 20 | + | for i := range publications { | |
| 21 | + | name := fmt.Sprintf("named-%03d", i) | |
| 22 | + | publications[i] = repositoryPublication(t, name, 1, []byte(name)) | |
| 23 | + | if err := repo.Publish(context.Background(), publications[i]); err != nil { | |
| 24 | + | t.Fatalf("initial Publish(%q): %v", name, err) | |
| 25 | + | } | |
| 26 | + | } | |
| 27 | + | ||
| 28 | + | var wg sync.WaitGroup | |
| 29 | + | for _, publication := range publications { | |
| 30 | + | publication := publication | |
| 31 | + | wg.Add(2) | |
| 32 | + | go func() { | |
| 33 | + | defer wg.Done() | |
| 34 | + | if err := repo.Publish(context.Background(), publication); err != nil { | |
| 35 | + | t.Errorf("Publish(%q): %v", publication.Name, err) | |
| 36 | + | } | |
| 37 | + | }() | |
| 38 | + | go func() { | |
| 39 | + | defer wg.Done() | |
| 40 | + | if err := repo.Delete(context.Background(), publication.Name); err != nil { | |
| 41 | + | t.Errorf("Delete(%q): %v", publication.Name, err) | |
| 42 | + | } | |
| 43 | + | }() | |
| 44 | + | } | |
| 45 | + | wg.Wait() | |
| 46 | + | ||
| 47 | + | locks, epochs := repo.sessionStateCounts() | |
| 48 | + | if locks != 0 || epochs != 0 { | |
| 49 | + | t.Fatalf("retained session state = %d locks, %d epochs, want none", locks, epochs) | |
| 50 | + | } | |
| 51 | + | } | |
| 52 | + | ||
| 53 | + | func TestRepositoryBoundsAndEvictsPartialMaintenanceSessionState(t *testing.T) { | |
| 54 | + | repo := NewRepository(privateDir(t)) | |
| 55 | + | for _, key := range []string{"named-000", "named-001"} { | |
| 56 | + | generations := filepath.Join(repo.sessionPath(key), repositoryGenerations) | |
| 57 | + | for i := 0; i < maintenanceBatch+1; i++ { | |
| 58 | + | if err := os.MkdirAll(generations, 0o700); err != nil { | |
| 59 | + | t.Fatal(err) | |
| 60 | + | } | |
| 61 | + | if err := os.WriteFile(filepath.Join(generations, fmt.Sprintf("unclassified-%03d", i)), []byte("state"), 0o600); err != nil { | |
| 62 | + | t.Fatal(err) | |
| 63 | + | } | |
| 64 | + | } | |
| 65 | + | } | |
| 66 | + | if err := repo.Maintain(context.Background()); err != nil { | |
| 67 | + | t.Fatal(err) | |
| 68 | + | } | |
| 69 | + | locks, epochs := repo.sessionStateCounts() | |
| 70 | + | if locks != 1 || epochs != 0 { | |
| 71 | + | t.Fatalf("partial maintenance state = %d locks, %d epochs, want one lock and no epochs", locks, epochs) | |
| 72 | + | } | |
| 73 | + | ||
| 74 | + | ctx, cancel := context.WithCancel(context.Background()) | |
| 75 | + | cancel() | |
| 76 | + | if err := repo.Maintain(ctx); err != context.Canceled { | |
| 77 | + | t.Fatalf("canceled Maintain error = %v, want context canceled", err) | |
| 78 | + | } | |
| 79 | + | locks, epochs = repo.sessionStateCounts() | |
| 80 | + | if locks != 0 || epochs != 0 { | |
| 81 | + | t.Fatalf("session state after maintenance reset = %d locks, %d epochs, want none", locks, epochs) | |
| 82 | + | } | |
| 83 | + | } | |
| 84 | + | ||
| 85 | + | func TestRepositoryDeleteUsesDeterministicPendingQuarantine(t *testing.T) { | |
| 86 | + | repo := NewRepository(privateDir(t)) | |
| 87 | + | if err := repo.Publish(context.Background(), repositoryPublication(t, "named", 1, []byte("state"))); err != nil { | |
| 88 | + | t.Fatal(err) | |
| 89 | + | } | |
| 90 | + | sessions := filepath.Join(repo.dir, repositorySessionsDir) | |
| 91 | + | for i := 0; i < maxDirectoryTraversalEntries*2; i++ { | |
| 92 | + | if err := os.Mkdir(filepath.Join(sessions, fmt.Sprintf("unrelated-%05d", i)), 0o700); err != nil { | |
| 93 | + | t.Fatal(err) | |
| 94 | + | } | |
| 95 | + | } | |
| 96 | + | var checks atomic.Int32 | |
| 97 | + | repo.hooks.beforePendingQuarantineCheck = func(string) { checks.Add(1) } | |
| 98 | + | if err := repo.Delete(context.Background(), "named"); err != nil { | |
| 99 | + | t.Fatal(err) | |
| 100 | + | } | |
| 101 | + | if got := checks.Load(); got != 1 { | |
| 102 | + | t.Fatalf("pending quarantine checks = %d, want one deterministic lookup", got) | |
| 103 | + | } | |
| 104 | + | if _, err := os.Lstat(filepath.Join(sessions, deletingSessionName(sessionKey("named")))); err != nil { | |
| 105 | + | t.Fatalf("deterministic quarantine missing: %v", err) | |
| 106 | + | } | |
| 107 | + | } | |
| @@ -0,0 +1,107 @@ | |||||
| 1 | + | package snapshot | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "context" | |||
| 5 | + | "fmt" | |||
| 6 | + | "os" | |||
| 7 | + | "path/filepath" | |||
| 8 | + | "sync" | |||
| 9 | + | "sync/atomic" | |||
| 10 | + | "testing" | |||
| 11 | + | ||||
| 12 | + | "github.com/bnema/vev/internal/ports" | |||
| 13 | + | ) | |||
| 14 | + | ||||
| 15 | + | func TestRepositoryPublishDeleteManySessionsRaceFree(t *testing.T) { | |||
| 16 | + | repo := NewRepository(privateDir(t)) | |||
| 17 | + | const sessions = 128 | |||
| 18 | + | ||||
| 19 | + | publications := make([]ports.SnapshotPublication, sessions) | |||
| 20 | + | for i := range publications { | |||
| 21 | + | name := fmt.Sprintf("named-%03d", i) | |||
| 22 | + | publications[i] = repositoryPublication(t, name, 1, []byte(name)) | |||
| 23 | + | if err := repo.Publish(context.Background(), publications[i]); err != nil { | |||
| 24 | + | t.Fatalf("initial Publish(%q): %v", name, err) | |||
| 25 | + | } | |||
| 26 | + | } | |||
| 27 | + | ||||
| 28 | + | var wg sync.WaitGroup | |||
| 29 | + | for _, publication := range publications { | |||
| 30 | + | publication := publication | |||
| 31 | + | wg.Add(2) | |||
| 32 | + | go func() { | |||
| 33 | + | defer wg.Done() | |||
| 34 | + | if err := repo.Publish(context.Background(), publication); err != nil { | |||
| 35 | + | t.Errorf("Publish(%q): %v", publication.Name, err) | |||
| 36 | + | } | |||
| 37 | + | }() | |||
| 38 | + | go func() { | |||
| 39 | + | defer wg.Done() | |||
| 40 | + | if err := repo.Delete(context.Background(), publication.Name); err != nil { | |||
| 41 | + | t.Errorf("Delete(%q): %v", publication.Name, err) | |||
| 42 | + | } | |||
| 43 | + | }() | |||
| 44 | + | } | |||
| 45 | + | wg.Wait() | |||
| 46 | + | ||||
| 47 | + | locks, epochs := repo.sessionStateCounts() | |||
| 48 | + | if locks != 0 || epochs != 0 { | |||
| 49 | + | t.Fatalf("retained session state = %d locks, %d epochs, want none", locks, epochs) | |||
| 50 | + | } | |||
| 51 | + | } | |||
| 52 | + | ||||
| 53 | + | func TestRepositoryBoundsAndEvictsPartialMaintenanceSessionState(t *testing.T) { | |||
| 54 | + | repo := NewRepository(privateDir(t)) | |||
| 55 | + | for _, key := range []string{"named-000", "named-001"} { | |||
| 56 | + | generations := filepath.Join(repo.sessionPath(key), repositoryGenerations) | |||
| 57 | + | for i := 0; i < maintenanceBatch+1; i++ { | |||
| 58 | + | if err := os.MkdirAll(generations, 0o700); err != nil { | |||
| 59 | + | t.Fatal(err) | |||
| 60 | + | } | |||
| 61 | + | if err := os.WriteFile(filepath.Join(generations, fmt.Sprintf("unclassified-%03d", i)), []byte("state"), 0o600); err != nil { | |||
| 62 | + | t.Fatal(err) | |||
| 63 | + | } | |||
| 64 | + | } | |||
| 65 | + | } | |||
| 66 | + | if err := repo.Maintain(context.Background()); err != nil { | |||
| 67 | + | t.Fatal(err) | |||
| 68 | + | } | |||
| 69 | + | locks, epochs := repo.sessionStateCounts() | |||
| 70 | + | if locks != 1 || epochs != 0 { | |||
| 71 | + | t.Fatalf("partial maintenance state = %d locks, %d epochs, want one lock and no epochs", locks, epochs) | |||
| 72 | + | } | |||
| 73 | + | ||||
| 74 | + | ctx, cancel := context.WithCancel(context.Background()) | |||
| 75 | + | cancel() | |||
| 76 | + | if err := repo.Maintain(ctx); err != context.Canceled { | |||
| 77 | + | t.Fatalf("canceled Maintain error = %v, want context canceled", err) | |||
| 78 | + | } | |||
| 79 | + | locks, epochs = repo.sessionStateCounts() | |||
| 80 | + | if locks != 0 || epochs != 0 { | |||
| 81 | + | t.Fatalf("session state after maintenance reset = %d locks, %d epochs, want none", locks, epochs) | |||
| 82 | + | } | |||
| 83 | + | } | |||
| 84 | + | ||||
| 85 | + | func TestRepositoryDeleteUsesDeterministicPendingQuarantine(t *testing.T) { | |||
| 86 | + | repo := NewRepository(privateDir(t)) | |||
| 87 | + | if err := repo.Publish(context.Background(), repositoryPublication(t, "named", 1, []byte("state"))); err != nil { | |||
| 88 | + | t.Fatal(err) | |||
| 89 | + | } | |||
| 90 | + | sessions := filepath.Join(repo.dir, repositorySessionsDir) | |||
| 91 | + | for i := 0; i < maxDirectoryTraversalEntries*2; i++ { | |||
| 92 | + | if err := os.Mkdir(filepath.Join(sessions, fmt.Sprintf("unrelated-%05d", i)), 0o700); err != nil { | |||
| 93 | + | t.Fatal(err) | |||
| 94 | + | } | |||
| 95 | + | } | |||
| 96 | + | var checks atomic.Int32 | |||
| 97 | + | repo.hooks.beforePendingQuarantineCheck = func(string) { checks.Add(1) } | |||
| 98 | + | if err := repo.Delete(context.Background(), "named"); err != nil { | |||
| 99 | + | t.Fatal(err) | |||
| 100 | + | } | |||
| 101 | + | if got := checks.Load(); got != 1 { | |||
| 102 | + | t.Fatalf("pending quarantine checks = %d, want one deterministic lookup", got) | |||
| 103 | + | } | |||
| 104 | + | if _, err := os.Lstat(filepath.Join(sessions, deletingSessionName(sessionKey("named")))); err != nil { | |||
| 105 | + | t.Fatalf("deterministic quarantine missing: %v", err) | |||
| 106 | + | } | |||
| 107 | + | } | |||