Skip to content

Commit a76bef7

Browse files
ziflexziflex
committed
fix: close engine filesystem roots
1 parent 6f348f6 commit a76bef7

8 files changed

Lines changed: 374 additions & 17 deletions

‎engine.go‎

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -42,21 +42,33 @@ func New(setters ...Option) (*Engine, error) {
4242

4343
for _, m := range opts.modules {
4444
if err := m.Register(boot); err != nil {
45-
return nil, closeEngineOnError(err, boot.hooks.engine, boot.host.Network(), ownsNetwork)
45+
return nil, closeEngineOnError(
46+
err,
47+
boot.hooks.engine,
48+
boot.host.FileSystem(),
49+
boot.host.Network(),
50+
ownsNetwork,
51+
)
4652
}
4753
}
4854

4955
h, err := boot.host.Build()
5056
if err != nil {
51-
return nil, closeEngineOnError(err, boot.hooks.engine, boot.host.Network(), ownsNetwork)
57+
return nil, closeEngineOnError(
58+
err,
59+
boot.hooks.engine,
60+
boot.host.FileSystem(),
61+
boot.host.Network(),
62+
ownsNetwork,
63+
)
5264
}
5365

5466
hooks := boot.hooks.clone()
5567
// Run init hooks after bootstrap is finalized and before returning the engine.
5668
if err := hooks.engine.runInitHooks(); err != nil {
5769
initErr := fmt.Errorf("init hooks: %w", err)
5870

59-
return nil, closeEngineOnError(initErr, hooks.engine, h.network, ownsNetwork)
71+
return nil, closeEngineOnError(initErr, hooks.engine, h.fs, h.network, ownsNetwork)
6072
}
6173

6274
return &Engine{
@@ -162,9 +174,10 @@ func (e *Engine) Run(ctx context.Context, src *source.Source, opts ...SessionOpt
162174
return session.Run(ctx)
163175
}
164176

165-
// Close runs the engine close hooks and releases engine-scoped resources.
177+
// Close runs the engine close hooks and releases engine-scoped resources,
178+
// including the configured rooted filesystem and owned network idle connections.
166179
func (e *Engine) Close() error {
167-
return closeEngine(e.hooks.engine, e.host.network, e.ownsNetwork)
180+
return closeEngine(e.hooks.engine, e.host.fs, e.host.network, e.ownsNetwork)
168181
}
169182

170183
func (e *Engine) newPlan(prog *bytecode.Program) (*Plan, error) {

‎engine_filesystem_lifecycle_test.go‎

Lines changed: 174 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,174 @@
1+
package ferret
2+
3+
import (
4+
"errors"
5+
"io"
6+
"strings"
7+
"testing"
8+
9+
ferretfs "github.com/MontFerret/ferret/v2/pkg/fs"
10+
"github.com/MontFerret/ferret/v2/pkg/module"
11+
ferretnet "github.com/MontFerret/ferret/v2/pkg/net"
12+
)
13+
14+
type failingCloseFileSystem struct {
15+
ferretfs.FileSystem
16+
closeErr error
17+
}
18+
19+
func (f *failingCloseFileSystem) Close() error {
20+
closer, ok := f.FileSystem.(io.Closer)
21+
if !ok {
22+
return f.closeErr
23+
}
24+
25+
return errors.Join(f.closeErr, closer.Close())
26+
}
27+
28+
func TestEngineCloseClosesRootFileSystem(t *testing.T) {
29+
t.Parallel()
30+
31+
engine, err := New(WithFSRoot(t.TempDir()))
32+
if err != nil {
33+
t.Fatalf("new engine: %v", err)
34+
}
35+
36+
filesystem := engine.host.fs
37+
38+
if err := engine.Close(); err != nil {
39+
t.Fatalf("close engine: %v", err)
40+
}
41+
42+
if _, err := filesystem.Stat("."); err == nil {
43+
t.Fatal("expected engine filesystem to be closed")
44+
}
45+
}
46+
47+
func TestNewClosesRootFileSystemOnRegistrationFailure(t *testing.T) {
48+
t.Parallel()
49+
50+
registerErr := errors.New("register failed")
51+
var filesystem ferretfs.FileSystem
52+
mod := testModule{
53+
registerFn: func(boot module.Bootstrap) error {
54+
filesystem = boot.Host().FileSystem()
55+
56+
return registerErr
57+
},
58+
}
59+
60+
_, err := New(WithFSRoot(t.TempDir()), WithModules(mod))
61+
if !errors.Is(err, registerErr) {
62+
t.Fatalf("expected registration error, got %v", err)
63+
}
64+
65+
if _, err := filesystem.Stat("."); err == nil {
66+
t.Fatal("expected construction failure to close the filesystem")
67+
}
68+
}
69+
70+
func TestNewClosesRootFileSystemOnHostBuildFailure(t *testing.T) {
71+
t.Parallel()
72+
73+
var filesystem ferretfs.FileSystem
74+
mod := testModule{
75+
registerFn: func(boot module.Bootstrap) error {
76+
filesystem = boot.Host().FileSystem()
77+
boot.Host().Library().Function().A0().Add("FILESYSTEM_DUPLICATE_FN", testFn0)
78+
boot.Host().Library().Function().A0().Add("FILESYSTEM_DUPLICATE_FN", testFn0)
79+
80+
return nil
81+
},
82+
}
83+
84+
_, err := New(WithFSRoot(t.TempDir()), WithModules(mod))
85+
if err == nil {
86+
t.Fatal("expected host build failure")
87+
}
88+
89+
if _, err := filesystem.Stat("."); err == nil {
90+
t.Fatal("expected host build failure to close the filesystem")
91+
}
92+
}
93+
94+
func TestNewClosesRootFileSystemOnInitFailure(t *testing.T) {
95+
t.Parallel()
96+
97+
initErr := errors.New("init failed")
98+
var filesystem ferretfs.FileSystem
99+
mod := testModule{
100+
registerFn: func(boot module.Bootstrap) error {
101+
filesystem = boot.Host().FileSystem()
102+
103+
return nil
104+
},
105+
}
106+
107+
_, err := New(
108+
WithFSRoot(t.TempDir()),
109+
WithModules(mod),
110+
WithEngineInitHook(func() error {
111+
return initErr
112+
}),
113+
)
114+
if !errors.Is(err, initErr) {
115+
t.Fatalf("expected init error, got %v", err)
116+
}
117+
118+
if _, err := filesystem.Stat("."); err == nil {
119+
t.Fatal("expected init failure to close the filesystem")
120+
}
121+
}
122+
123+
func TestNewJoinsConstructionHookAndFileSystemCloseErrors(t *testing.T) {
124+
t.Parallel()
125+
126+
registerErr := errors.New("register failed")
127+
hookErr := errors.New("hook close failed")
128+
filesystemErr := errors.New("filesystem close failed")
129+
client := &recordingHTTPClient{}
130+
mod := testModule{
131+
registerFn: func(boot module.Bootstrap) error {
132+
internal, ok := boot.(*bootstrap)
133+
if !ok {
134+
t.Fatalf("expected internal bootstrap, got %T", boot)
135+
}
136+
137+
internal.host.fs = &failingCloseFileSystem{
138+
FileSystem: internal.host.fs,
139+
closeErr: filesystemErr,
140+
}
141+
internal.host.network = mustNewTestNetwork(t, ferretnet.WithHTTPClient(client))
142+
boot.Hooks().Engine().OnClose(func() error {
143+
return hookErr
144+
})
145+
146+
return registerErr
147+
},
148+
}
149+
150+
_, err := New(WithFSRoot(t.TempDir()), WithModules(mod))
151+
if !errors.Is(err, registerErr) {
152+
t.Fatalf("expected registration error, got %v", err)
153+
}
154+
155+
if !errors.Is(err, hookErr) {
156+
t.Fatalf("expected hook close error, got %v", err)
157+
}
158+
159+
if !errors.Is(err, filesystemErr) {
160+
t.Fatalf("expected filesystem close error, got %v", err)
161+
}
162+
163+
if !strings.Contains(err.Error(), "close hooks") {
164+
t.Fatalf("expected close hooks label, got %v", err)
165+
}
166+
167+
if !strings.Contains(err.Error(), "close filesystem") {
168+
t.Fatalf("expected close filesystem label, got %v", err)
169+
}
170+
171+
if got := client.idleCloseCount(); got != 1 {
172+
t.Fatalf("expected network cleanup after close errors, got %d calls", got)
173+
}
174+
}

‎engine_filesystem_lifecycle_windows_test.go‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
//go:build windows
2+
3+
package ferret
4+
5+
import (
6+
"errors"
7+
"os"
8+
"path/filepath"
9+
"testing"
10+
11+
"github.com/MontFerret/ferret/v2/pkg/module"
12+
)
13+
14+
func TestEngineCloseReleasesRootDirectoryOnWindows(t *testing.T) {
15+
root := filepath.Join(t.TempDir(), "workspace")
16+
if err := os.Mkdir(root, 0o700); err != nil {
17+
t.Fatalf("create root: %v", err)
18+
}
19+
20+
engine, err := New(WithFSRoot(root))
21+
if err != nil {
22+
t.Fatalf("new engine: %v", err)
23+
}
24+
25+
if err := engine.Close(); err != nil {
26+
t.Fatalf("close engine: %v", err)
27+
}
28+
29+
if err := os.Remove(root); err != nil {
30+
t.Fatalf("remove root after engine close: %v", err)
31+
}
32+
}
33+
34+
func TestNewReleasesRootDirectoryOnRegistrationFailureOnWindows(t *testing.T) {
35+
root := filepath.Join(t.TempDir(), "workspace")
36+
if err := os.Mkdir(root, 0o700); err != nil {
37+
t.Fatalf("create root: %v", err)
38+
}
39+
40+
registerErr := errors.New("register failed")
41+
mod := testModule{
42+
registerFn: func(module.Bootstrap) error {
43+
return registerErr
44+
},
45+
}
46+
47+
_, err := New(WithFSRoot(root), WithModules(mod))
48+
if !errors.Is(err, registerErr) {
49+
t.Fatalf("expected registration error, got %v", err)
50+
}
51+
52+
if err := os.Remove(root); err != nil {
53+
t.Fatalf("remove root after construction failure: %v", err)
54+
}
55+
}

‎engine_helpers.go‎

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,32 +3,59 @@ package ferret
33
import (
44
"errors"
55
"fmt"
6+
"io"
67

8+
ferretfs "github.com/MontFerret/ferret/v2/pkg/fs"
79
ferretnet "github.com/MontFerret/ferret/v2/pkg/net"
810
)
911

10-
func closeEngine(hooks *engineHookRegistry, network ferretnet.Network, ownsNetwork bool) error {
11-
closeErr := hooks.runCloseHooks()
12+
func closeEngine(
13+
hooks *engineHookRegistry,
14+
filesystem ferretfs.FileSystem,
15+
network ferretnet.Network,
16+
ownsNetwork bool,
17+
) error {
18+
hookErr := hooks.runCloseHooks()
19+
filesystemErr := closeFileSystem(filesystem)
1220

1321
if ownsNetwork {
1422
ferretnet.CloseIdleNetworkConnections(network)
1523
}
1624

17-
if closeErr != nil {
18-
return errors.Join(closeErr, fmt.Errorf("close hooks: %w", closeErr))
25+
if hookErr != nil {
26+
hookErr = errors.Join(hookErr, fmt.Errorf("close hooks: %w", hookErr))
1927
}
2028

21-
return nil
29+
return errors.Join(hookErr, filesystemErr)
2230
}
2331

24-
func closeEngineOnError(err error, hooks *engineHookRegistry, network ferretnet.Network, ownsNetwork bool) error {
32+
func closeEngineOnError(
33+
err error,
34+
hooks *engineHookRegistry,
35+
filesystem ferretfs.FileSystem,
36+
network ferretnet.Network,
37+
ownsNetwork bool,
38+
) error {
2539
if err != nil {
26-
closeErr := closeEngine(hooks, network, ownsNetwork)
40+
closeErr := closeEngine(hooks, filesystem, network, ownsNetwork)
2741

2842
if closeErr != nil {
29-
return errors.Join(err, fmt.Errorf("close hooks: %w", closeErr))
43+
return errors.Join(err, fmt.Errorf("close engine: %w", closeErr))
3044
}
3145
}
3246

3347
return err
3448
}
49+
50+
func closeFileSystem(filesystem ferretfs.FileSystem) error {
51+
closer, ok := filesystem.(io.Closer)
52+
if !ok || closer == nil {
53+
return nil
54+
}
55+
56+
if err := closer.Close(); err != nil {
57+
return fmt.Errorf("close filesystem: %w", err)
58+
}
59+
60+
return nil
61+
}

‎engine_lifecycle_benchmark_test.go‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
package ferret
2+
3+
import (
4+
"io"
5+
"testing"
6+
)
7+
8+
func BenchmarkEngineFSRootLifecycle(b *testing.B) {
9+
root := b.TempDir()
10+
11+
b.ReportAllocs()
12+
b.ResetTimer()
13+
14+
for range b.N {
15+
engine, err := New(WithFSRoot(root))
16+
if err != nil {
17+
b.Fatalf("new engine: %v", err)
18+
}
19+
20+
if err := engine.Close(); err != nil {
21+
b.Fatalf("close engine: %v", err)
22+
}
23+
24+
// Keep a direct close outside the measured interval so a lifecycle
25+
// regression cannot make the benchmark accumulate descriptors.
26+
b.StopTimer()
27+
if closer, ok := engine.host.fs.(io.Closer); ok {
28+
if err := closer.Close(); err != nil {
29+
b.Fatalf("close filesystem: %v", err)
30+
}
31+
}
32+
b.StartTimer()
33+
}
34+
}

0 commit comments

Comments
 (0)