Skip to content

Commit 3a43454

Browse files
ziflexziflex
authored
fix: close engine filesystem roots (#996)
* fix: close engine filesystem roots * refactor: make filesystem lifecycle explicit
1 parent 6f348f6 commit 3a43454

12 files changed

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

‎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: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,31 +4,52 @@ import (
44
"errors"
55
"fmt"
66

7+
ferretfs "github.com/MontFerret/ferret/v2/pkg/fs"
78
ferretnet "github.com/MontFerret/ferret/v2/pkg/net"
89
)
910

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

1320
if ownsNetwork {
1421
ferretnet.CloseIdleNetworkConnections(network)
1522
}
1623

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

21-
return nil
28+
return errors.Join(hookErr, filesystemErr)
2229
}
2330

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

2841
if closeErr != nil {
29-
return errors.Join(err, fmt.Errorf("close hooks: %w", closeErr))
42+
return errors.Join(err, fmt.Errorf("close engine: %w", closeErr))
3043
}
3144
}
3245

3346
return err
3447
}
48+
49+
func closeFileSystem(filesystem ferretfs.FileSystem) error {
50+
if err := filesystem.Close(); err != nil {
51+
return fmt.Errorf("close filesystem: %w", err)
52+
}
53+
54+
return nil
55+
}

‎engine_lifecycle_benchmark_test.go‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
package ferret
2+
3+
import "testing"
4+
5+
func BenchmarkEngineFSRootLifecycle(b *testing.B) {
6+
root := b.TempDir()
7+
8+
b.ReportAllocs()
9+
b.ResetTimer()
10+
11+
for range b.N {
12+
engine, err := New(WithFSRoot(root))
13+
if err != nil {
14+
b.Fatalf("new engine: %v", err)
15+
}
16+
17+
if err := engine.Close(); err != nil {
18+
b.Fatalf("close engine: %v", err)
19+
}
20+
}
21+
}

‎host.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package ferret
22

33
import (
4+
"errors"
45
"fmt"
56

67
"github.com/MontFerret/ferret/v2/pkg/encoding"
@@ -41,7 +42,9 @@ func newHostContext(opts *options) (*hostContext, error) {
4142
if network == nil {
4243
network, err = ferretnet.New()
4344
if err != nil {
44-
return nil, fmt.Errorf("network: %w", err)
45+
networkErr := fmt.Errorf("network: %w", err)
46+
47+
return nil, errors.Join(networkErr, closeFileSystem(rootFs))
4548
}
4649
}
4750

0 commit comments

Comments
 (0)