From 673902b14458f32fbf47efa3757279872bdfcb7e Mon Sep 17 00:00:00 2001 From: Runxi Yu Date: Sat, 21 Feb 2026 15:29:40 +0800 Subject: repository, {ref,object}store: Make stores own their roots --- objectstore/loose/store.go | 7 +-- objectstore/packed/store.go | 13 +++-- refstore/loose/store.go | 4 +- refstore/reftable/store.go | 7 ++- repository/repository.go | 130 ++++++++++++++++---------------------------- 5 files changed, 64 insertions(+), 97 deletions(-) diff --git a/objectstore/loose/store.go b/objectstore/loose/store.go index b0f64971..05459a6c 100644 --- a/objectstore/loose/store.go +++ b/objectstore/loose/store.go @@ -9,11 +9,11 @@ import ( // Store reads loose Git objects from an objects directory root. // -// Store does not own root. Callers are responsible for closing root. +// Store owns root and closes it in Close. type Store struct { // root is the objects directory capability used for all object file access. // Object files are opened by relative paths like "/". - // Store does not own this root. + // Store owns this root. root *os.Root // algo is the expected object ID algorithm for lookups. algo objectid.Algorithm @@ -32,6 +32,5 @@ func New(root *os.Root, algo objectid.Algorithm) (*Store, error) { // Close releases resources associated with the backend. func (store *Store) Close() error { - _ = store - return nil + return store.root.Close() } diff --git a/objectstore/packed/store.go b/objectstore/packed/store.go index fc2e1c10..ace11782 100644 --- a/objectstore/packed/store.go +++ b/objectstore/packed/store.go @@ -12,7 +12,7 @@ import ( // Store reads Git objects from pack/index files under an objects/pack root. // -// Store does not own root. Callers are responsible for closing root. +// Store owns root and closes it in Close. type Store struct { // root is the objects/pack capability used for all file access. root *os.Root @@ -65,10 +65,9 @@ func (store *Store) Close() error { return nil } store.closed = true + root := store.root packs := store.packs - store.packs = make(map[string]*packFile) indexes := store.indexes - store.indexes = nil store.stateMu.Unlock() var closeErr error @@ -86,10 +85,12 @@ func (store *Store) Close() error { } } store.cacheMu.Lock() - if store.deltaCache != nil { - store.deltaCache.clear() - } + store.deltaCache.clear() store.cacheMu.Unlock() + + if err := root.Close(); err != nil && closeErr == nil { + closeErr = err + } return closeErr } diff --git a/refstore/loose/store.go b/refstore/loose/store.go index 8eb485e9..e4dc3a34 100644 --- a/refstore/loose/store.go +++ b/refstore/loose/store.go @@ -10,7 +10,7 @@ import ( // Store reads loose references from a repository root. // -// Store does not own root. Callers are responsible for closing root. +// Store owns root and closes it in Close. type Store struct { // root is the repository root capability. root *os.Root @@ -33,5 +33,5 @@ func New(root *os.Root, algo objectid.Algorithm) (*Store, error) { // Close releases resources associated with the backend. func (store *Store) Close() error { - return nil + return store.root.Close() } diff --git a/refstore/reftable/store.go b/refstore/reftable/store.go index ac730a4b..7c02c157 100644 --- a/refstore/reftable/store.go +++ b/refstore/reftable/store.go @@ -15,7 +15,7 @@ import ( // Store reads references from a reftable stack rooted at $GIT_DIR/reftable. // -// Store does not own root. Callers are responsible for closing root. +// Store owns root and closes it in Close. type Store struct { // root is the reftable directory capability. root *os.Root @@ -53,8 +53,8 @@ func (store *Store) Close() error { return nil } store.closed = true + root := store.root tables := store.tables - store.tables = nil store.stateMu.Unlock() var closeErr error @@ -66,6 +66,9 @@ func (store *Store) Close() error { closeErr = err } } + if err := root.Close(); err != nil && closeErr == nil { + closeErr = err + } return closeErr } diff --git a/repository/repository.go b/repository/repository.go index 1e2834a3..a1efe92f 100644 --- a/repository/repository.go +++ b/repository/repository.go @@ -24,11 +24,6 @@ import ( // Open expects path to be the Git directory itself: // a bare repository root or a non-bare ".git" directory. type Repository struct { - root *os.Root - objectsRoot *os.Root - packRoot *os.Root - reftableRoot *os.Root - config *config.Config algo objectid.Algorithm @@ -38,19 +33,20 @@ type Repository struct { // Open opens a repository and wires object/ref stores from its on-disk format. func Open(path string) (repo *Repository, err error) { - root, err := os.OpenRoot(path) + setupRoot, err := os.OpenRoot(path) if err != nil { return nil, err } + defer func() { _ = setupRoot.Close() }() - repo = &Repository{root: root} + repo = &Repository{} defer func() { if err != nil { _ = repo.Close() } }() - cfg, err := parseRepositoryConfig(root) + cfg, err := parseRepositoryConfig(setupRoot) if err != nil { return nil, err } @@ -62,20 +58,17 @@ func Open(path string) (repo *Repository, err error) { } repo.algo = algo - objects, objectsRoot, packRoot, err := openObjectStore(root, algo) + objects, err := openObjectStore(path, algo) if err != nil { return nil, err } repo.objects = objects - repo.objectsRoot = objectsRoot - repo.packRoot = packRoot - refs, reftableRoot, err := openRefStore(root, algo) + refs, err := openRefStore(path, algo) if err != nil { return nil, err } repo.refs = refs - repo.reftableRoot = reftableRoot return repo, nil } @@ -112,38 +105,11 @@ func (repo *Repository) Close() error { if err := repo.refs.Close(); err != nil { errs = append(errs, err) } - repo.refs = nil } if repo.objects != nil { if err := repo.objects.Close(); err != nil { errs = append(errs, err) } - repo.objects = nil - } - - if repo.reftableRoot != nil { - if err := repo.reftableRoot.Close(); err != nil { - errs = append(errs, err) - } - repo.reftableRoot = nil - } - if repo.packRoot != nil { - if err := repo.packRoot.Close(); err != nil { - errs = append(errs, err) - } - repo.packRoot = nil - } - if repo.objectsRoot != nil { - if err := repo.objectsRoot.Close(); err != nil { - errs = append(errs, err) - } - repo.objectsRoot = nil - } - if repo.root != nil { - if err := repo.root.Close(); err != nil { - errs = append(errs, err) - } - repo.root = nil } return errors.Join(errs...) @@ -175,11 +141,18 @@ func detectObjectAlgorithm(cfg *config.Config) (objectid.Algorithm, error) { return algo, nil } -func openObjectStore(root *os.Root, algo objectid.Algorithm) (out objectstore.Store, objectsRoot *os.Root, packRoot *os.Root, err error) { - objectsRoot, err = root.OpenRoot("objects") +func openObjectStore(path string, algo objectid.Algorithm) (out objectstore.Store, err error) { + repoRoot, err := os.OpenRoot(path) + if err != nil { + return nil, fmt.Errorf("repository: open root: %w", err) + } + defer func() { _ = repoRoot.Close() }() + + objectsRoot, err := repoRoot.OpenRoot("objects") if err != nil { - return nil, nil, nil, fmt.Errorf("repository: open objects: %w", err) + return nil, fmt.Errorf("repository: open objects: %w", err) } + var packRoot *os.Root defer func() { if err != nil { if out != nil { @@ -194,7 +167,7 @@ func openObjectStore(root *os.Root, algo objectid.Algorithm) (out objectstore.St looseStore, err := objectloose.New(objectsRoot, algo) if err != nil { - return nil, nil, nil, err + return nil, err } backends := []objectstore.Store{looseStore} @@ -203,77 +176,68 @@ func openObjectStore(root *os.Root, algo objectid.Algorithm) (out objectstore.St var packedStore *objectpacked.Store packedStore, err = objectpacked.New(packRoot, algo) if err != nil { - return nil, nil, nil, err + return nil, err } backends = append(backends, packedStore) } else if !errors.Is(err, os.ErrNotExist) { - return nil, nil, nil, fmt.Errorf("repository: open objects/pack: %w", err) + return nil, fmt.Errorf("repository: open objects/pack: %w", err) } err = nil out = objectchain.New(backends...) - return out, objectsRoot, packRoot, nil + return out, nil } -func openRefStore(root *os.Root, algo objectid.Algorithm) (out refstore.Store, reftableRoot *os.Root, err error) { - var closePackedStore refstore.Store - defer func() { - if err != nil { - if out != nil { - _ = out.Close() - } - if closePackedStore != nil { - _ = closePackedStore.Close() - } - if reftableRoot != nil { - _ = reftableRoot.Close() - } - } - }() +func openRefStore(path string, algo objectid.Algorithm) (out refstore.Store, err error) { + metaRoot, err := os.OpenRoot(path) + if err != nil { + return nil, fmt.Errorf("repository: open root: %w", err) + } + defer func() { _ = metaRoot.Close() }() - hasReftable, err := hasReftableStack(root) + hasReftable, err := hasReftableStack(metaRoot) if err != nil { - return nil, nil, err + return nil, err } if hasReftable { - reftableRoot, err = root.OpenRoot("reftable") + reftableRoot, err := metaRoot.OpenRoot("reftable") if err != nil { - return nil, nil, fmt.Errorf("repository: open reftable: %w", err) + return nil, fmt.Errorf("repository: open reftable: %w", err) } - var reftableStore *reftable.Store - reftableStore, err = reftable.New(reftableRoot, algo) + reftableStore, err := reftable.New(reftableRoot, algo) if err != nil { - return nil, nil, err + _ = reftableRoot.Close() + return nil, err } - err = nil - out = reftableStore - return reftableStore, reftableRoot, nil + return reftableStore, nil } - looseStore, err := refloose.New(root, algo) + looseRoot, err := os.OpenRoot(path) if err != nil { - return nil, nil, err + return nil, fmt.Errorf("repository: open root for loose refs: %w", err) + } + looseStore, err := refloose.New(looseRoot, algo) + if err != nil { + _ = looseRoot.Close() + return nil, err } backends := []refstore.Store{looseStore} - packedRefsFile, err := root.Open("packed-refs") + packedRefsFile, err := metaRoot.Open("packed-refs") if err == nil { packedStore, packedErr := refpacked.New(packedRefsFile, algo) _ = packedRefsFile.Close() if packedErr != nil { - err = packedErr - return nil, nil, err + _ = looseStore.Close() + return nil, packedErr } - closePackedStore = packedStore backends = append(backends, packedStore) } else if !errors.Is(err, os.ErrNotExist) { - return nil, nil, fmt.Errorf("repository: open packed-refs: %w", err) + _ = looseStore.Close() + return nil, fmt.Errorf("repository: open packed-refs: %w", err) } - err = nil - out = refchain.New(backends...) - closePackedStore = nil - return out, nil, nil + return refchain.New(backends...), nil } func hasReftableStack(root *os.Root) (bool, error) { -- cgit v1.3.1-10-gc9f91