diff --git a/embedfs/concurrent_readat_test.go b/embedfs/concurrent_readat_test.go new file mode 100644 index 0000000..d1aca45 --- /dev/null +++ b/embedfs/concurrent_readat_test.go @@ -0,0 +1,51 @@ +package embedfs + +import ( + "embed" + "sync" + "testing" + + "github.com/stretchr/testify/require" +) + +//go:embed testdata/concurrent.bin +var concurrentFS embed.FS + +// TestFileConcurrentReadAt asserts that concurrent ReadAt calls on a +// shared embedfs file handle are safe under -race and return correct +// per-byte results. embedfs wraps bytes.Reader, which is already +// concurrent-safe for ReadAt; this test pins the billy.File.ReadAt +// contract so future changes to the wrapper cannot regress it. +func TestFileConcurrentReadAt(t *testing.T) { + t.Parallel() + + want, err := concurrentFS.ReadFile("testdata/concurrent.bin") + require.NoError(t, err) + require.Equal(t, 4096, len(want)) + + fs := New(&concurrentFS) + f, err := fs.Open("testdata/concurrent.bin") + require.NoError(t, err) + t.Cleanup(func() { _ = f.Close() }) + + const workers = 8 + const iters = 200 + const bufLen = 64 + + var wg sync.WaitGroup + wg.Add(workers) + for w := range workers { + go func() { + defer wg.Done() + buf := make([]byte, bufLen) + for i := range iters { + off := int64((w*131 + i*257) % (len(want) - bufLen)) + n, err := f.ReadAt(buf, off) + require.NoError(t, err) + require.Equal(t, bufLen, n) + require.Equal(t, want[off:off+int64(n)], buf[:n]) + } + }() + } + wg.Wait() +} diff --git a/embedfs/embed_test.go b/embedfs/embed_test.go index 8b831fe..08522a2 100644 --- a/embedfs/embed_test.go +++ b/embedfs/embed_test.go @@ -283,7 +283,7 @@ func TestReadDir(t *testing.T) { name: "testdataDir w/ path", path: "testdata", fs: &testdataDir, - want: []string{"empty.txt", "empty2.txt"}, + want: []string{"concurrent.bin", "empty.txt", "empty2.txt"}, }, { name: "testdataDir return no dir names", diff --git a/embedfs/testdata/concurrent.bin b/embedfs/testdata/concurrent.bin new file mode 100644 index 0000000..18a43fa Binary files /dev/null and b/embedfs/testdata/concurrent.bin differ diff --git a/fs.go b/fs.go index 653b4d3..fe7cb4d 100644 --- a/fs.go +++ b/fs.go @@ -177,6 +177,12 @@ type File interface { Name() string io.Writer io.WriterAt + // ReadAt must be safe for concurrent calls from multiple + // goroutines on the same handle, matching the [io.ReaderAt] + // contract documented in package io. Callers performing + // concurrent random-access reads on a shared File rely on + // this; backings that cannot honour it must not satisfy + // [billy.File]. io.ReaderAt io.Seeker // Truncate the file. diff --git a/memfs/concurrent_readat_test.go b/memfs/concurrent_readat_test.go new file mode 100644 index 0000000..6ae5058 --- /dev/null +++ b/memfs/concurrent_readat_test.go @@ -0,0 +1,54 @@ +package memfs + +import ( + "sync" + "testing" + + "github.com/stretchr/testify/require" +) + +// TestFileConcurrentReadAt asserts that concurrent ReadAt calls on a +// shared memfs file handle are safe under -race and return correct +// per-byte results. memfs is the in-memory billy backing that go-git +// and other consumers rely on for tests; the billy.File.ReadAt +// contract documents concurrent-safety as required, and this test +// pins memfs's compliance. +func TestFileConcurrentReadAt(t *testing.T) { + t.Parallel() + + const size = 64 * 1024 + want := make([]byte, size) + for i := range want { + want[i] = byte(i % 251) + } + + fs := New() + f, err := fs.Create("data") + require.NoError(t, err) + _, err = f.Write(want) + require.NoError(t, err) + require.NoError(t, f.Close()) + + rf, err := fs.Open("data") + require.NoError(t, err) + t.Cleanup(func() { _ = rf.Close() }) + + const workers = 8 + const iters = 200 + var wg sync.WaitGroup + wg.Add(workers) + for w := range workers { + go func() { + defer wg.Done() + buf := make([]byte, 1024) + for i := range iters { + off := int64((w*131 + i*257) % (len(want) - len(buf))) + n, err := rf.ReadAt(buf, off) + require.NoError(t, err) + require.Equal(t, len(buf), n) + require.Equal(t, want[off:off+int64(n)], buf[:n]) + } + }() + } + wg.Wait() +} diff --git a/osfs/mmap_file.go b/osfs/mmap_file.go new file mode 100644 index 0000000..c128789 --- /dev/null +++ b/osfs/mmap_file.go @@ -0,0 +1,177 @@ +//go:build darwin || linux + +package osfs + +import ( + "errors" + "io" + "math" + "os" + "runtime" + "sync" + + "golang.org/x/sys/unix" +) + +// mmapFile is a billy.File backed by a read-only memory map. It is +// returned from BoundOS/RootOS.OpenFile when the filesystem was +// constructed with [WithMmap] and the file is opened without write +// flags. Read and Seek track a real cursor over the mapped bytes, +// ReadAt is concurrent-safe (multiple goroutines may call it in +// parallel against the same handle) and serialised against Close +// via an RWMutex so munmap cannot run while a read is in flight. +// Write/WriteAt/Truncate return [os.ErrPermission] — the file is +// read-only by construction. +type mmapFile struct { + f *os.File + data []byte + name string + + mu sync.RWMutex + cursor int64 + closed bool +} + +// newMmapFile maps f read-only and returns an [*mmapFile] that owns +// f. On success the returned handle is responsible for closing the +// underlying [*os.File] via [(*mmapFile).Close]. +// +// If mmap is unavailable for this particular file (zero size, size +// beyond platform int, mmap rejected by the kernel for pipes/devices +// etc.) the function returns [errMmapUnavailable] without closing f +// so the caller can fall back to a regular [*file] wrapper. +// +// Any other error (e.g. fstat failing) is propagated as-is and f is +// closed before returning — the caller must not use it. +func newMmapFile(f *os.File, name string) (*mmapFile, error) { + info, err := f.Stat() + if err != nil { + _ = f.Close() + return nil, err + } + + size := info.Size() + if size <= 0 || size > int64(math.MaxInt) { + // unix.Mmap rejects size 0, and 32-bit platforms can't + // represent very large mappings as an int. Either case + // is fine for the regular fd wrapper. + return nil, errMmapUnavailable + } + + data, err := unix.Mmap(int(f.Fd()), 0, int(size), unix.PROT_READ, unix.MAP_SHARED) + if err != nil { + // Many failure modes here are legitimate (pipes, devices, + // FS quirks). Defer to the fd wrapper. + return nil, errMmapUnavailable + } + + m := &mmapFile{f: f, data: data, name: name} + // Belt and braces for callers that forget to Close: the runtime + // will munmap and close the fd when m becomes unreachable. Close + // clears this finalizer on the orderly path. + runtime.SetFinalizer(m, (*mmapFile).Close) + return m, nil +} + +func (m *mmapFile) Name() string { return m.name } + +// Stat returns the underlying *os.File's FileInfo unchanged so that +// f.Stat().Name() matches the basename returned by the fd-backed +// *file across both backings. +func (m *mmapFile) Stat() (os.FileInfo, error) { + return m.f.Stat() +} + +// Read implements [io.Reader]. It holds the write lock because it +// mutates the shared cursor; concurrent Read+Read would otherwise +// race on m.cursor even though both could read m.data under RLock. +// Random-access callers should use ReadAt, which is the parallel API. +func (m *mmapFile) Read(p []byte) (int, error) { + if len(p) == 0 { + return 0, nil + } + m.mu.Lock() + defer m.mu.Unlock() + if m.closed { + return 0, os.ErrClosed + } + if m.cursor >= int64(len(m.data)) { + return 0, io.EOF + } + n := copy(p, m.data[m.cursor:]) + m.cursor += int64(n) + return n, nil +} + +func (m *mmapFile) ReadAt(p []byte, off int64) (int, error) { + if len(p) == 0 { + return 0, nil + } + m.mu.RLock() + defer m.mu.RUnlock() + if m.closed { + return 0, os.ErrClosed + } + if off < 0 { + return 0, &os.PathError{Op: "readat", Path: m.name, Err: errors.New("negative offset")} + } + if off >= int64(len(m.data)) { + return 0, io.EOF + } + n := copy(p, m.data[off:]) + if n < len(p) { + return n, io.EOF + } + return n, nil +} + +func (m *mmapFile) Seek(offset int64, whence int) (int64, error) { + m.mu.Lock() + defer m.mu.Unlock() + if m.closed { + return 0, os.ErrClosed + } + var abs int64 + switch whence { + case io.SeekStart: + abs = offset + case io.SeekCurrent: + abs = m.cursor + offset + case io.SeekEnd: + abs = int64(len(m.data)) + offset + default: + return 0, &os.PathError{Op: "seek", Path: m.name, Err: errors.New("invalid whence")} + } + if abs < 0 { + return 0, &os.PathError{Op: "seek", Path: m.name, Err: errors.New("negative position")} + } + m.cursor = abs + return abs, nil +} + +func (m *mmapFile) Write(p []byte) (int, error) { + return 0, &os.PathError{Op: "write", Path: m.name, Err: os.ErrPermission} +} + +func (m *mmapFile) WriteAt(p []byte, off int64) (int, error) { + return 0, &os.PathError{Op: "writeat", Path: m.name, Err: os.ErrPermission} +} + +func (m *mmapFile) Truncate(size int64) error { + return &os.PathError{Op: "truncate", Path: m.name, Err: os.ErrPermission} +} + +func (m *mmapFile) Close() error { + m.mu.Lock() + defer m.mu.Unlock() + if m.closed { + return os.ErrClosed + } + m.closed = true + runtime.SetFinalizer(m, nil) + + munmapErr := unix.Munmap(m.data) + m.data = nil + closeErr := m.f.Close() + return errors.Join(munmapErr, closeErr) +} diff --git a/osfs/mmap_file_assert_other_test.go b/osfs/mmap_file_assert_other_test.go new file mode 100644 index 0000000..26a7018 --- /dev/null +++ b/osfs/mmap_file_assert_other_test.go @@ -0,0 +1,18 @@ +//go:build !darwin && !linux && !wasm + +package osfs + +import ( + "testing" + + "github.com/go-git/go-billy/v6" + "github.com/stretchr/testify/require" +) + +func assertMmapBackingWhenAvailable(t *testing.T, f billy.File) { + t.Helper() + // mmap is unavailable on this platform; WithMmap is honoured as + // best-effort and falls through to the fd-backed wrapper. + _, ok := f.(*file) + require.True(t, ok, "platform has no mmap path, expected *file, got %T", f) +} diff --git a/osfs/mmap_file_assert_test.go b/osfs/mmap_file_assert_test.go new file mode 100644 index 0000000..d0e2fc9 --- /dev/null +++ b/osfs/mmap_file_assert_test.go @@ -0,0 +1,20 @@ +//go:build darwin || linux + +package osfs + +import ( + "testing" + + "github.com/go-git/go-billy/v6" + "github.com/stretchr/testify/require" +) + +func assertMmapBackingWhenAvailable(t *testing.T, f billy.File) { + t.Helper() + _, ok := f.(*mmapFile) + require.True(t, ok, "WithMmap should select *mmapFile on this platform, got %T", f) +} + +// Ensure *mmapFile satisfies billy.File at compile-time on platforms +// where it has a real implementation. +var _ billy.File = (*mmapFile)(nil) diff --git a/osfs/mmap_file_other.go b/osfs/mmap_file_other.go new file mode 100644 index 0000000..ea5987d --- /dev/null +++ b/osfs/mmap_file_other.go @@ -0,0 +1,28 @@ +//go:build !darwin && !linux && !js + +package osfs + +import "os" + +// mmapFile is a stub on platforms without mmap support. The +// real implementation lives in mmap_file.go (darwin || linux); +// here the type exists solely so the wrapOpenedFile return +// path that constructs it still type-checks. newMmapFile +// always returns [errMmapUnavailable], so a value of this type +// is never actually constructed at runtime — the method bodies +// below are unreachable. +type mmapFile struct{} + +func newMmapFile(_ *os.File, _ string) (*mmapFile, error) { + return nil, errMmapUnavailable +} + +func (m *mmapFile) Name() string { return "" } +func (m *mmapFile) Stat() (os.FileInfo, error) { return nil, os.ErrInvalid } +func (m *mmapFile) Read(p []byte) (int, error) { return 0, os.ErrInvalid } +func (m *mmapFile) ReadAt(p []byte, off int64) (int, error) { return 0, os.ErrInvalid } +func (m *mmapFile) Write(p []byte) (int, error) { return 0, os.ErrInvalid } +func (m *mmapFile) WriteAt(p []byte, off int64) (int, error) { return 0, os.ErrInvalid } +func (m *mmapFile) Seek(offset int64, whence int) (int64, error) { return 0, os.ErrInvalid } +func (m *mmapFile) Truncate(size int64) error { return os.ErrInvalid } +func (m *mmapFile) Close() error { return os.ErrInvalid } diff --git a/osfs/mmap_file_test.go b/osfs/mmap_file_test.go new file mode 100644 index 0000000..c1547e6 --- /dev/null +++ b/osfs/mmap_file_test.go @@ -0,0 +1,379 @@ +//go:build !wasm + +package osfs + +import ( + "io" + "os" + "path/filepath" + "sync" + "testing" + + "github.com/go-git/go-billy/v6" + "github.com/stretchr/testify/require" +) + +func writePattern(t *testing.T, path string, size int) []byte { + t.Helper() + buf := make([]byte, size) + for i := range buf { + buf[i] = byte(i % 251) + } + require.NoError(t, os.WriteFile(path, buf, 0o600)) + return buf +} + +func runConcurrentReadAt(t *testing.T, ra io.ReaderAt, want []byte) { + t.Helper() + const workers = 8 + const iters = 200 + var wg sync.WaitGroup + wg.Add(workers) + for w := range workers { + go func() { + defer wg.Done() + buf := make([]byte, 1024) + for i := range iters { + off := int64((w*131 + i*257) % (len(want) - len(buf))) + n, err := ra.ReadAt(buf, off) + require.NoError(t, err) + require.Equal(t, len(buf), n) + require.Equal(t, want[off:off+int64(n)], buf[:n]) + } + }() + } + wg.Wait() +} + +// modes drives the parametrised tests across both backings the +// platform offers: the default *file (fd-backed *os.File.ReadAt) +// and, on darwin/linux when WithMmap is passed, the *mmapFile. +// On platforms without mmap support the WithMmap row still runs +// but exercises the fd backing (WithMmap is best-effort). +var modes = []struct { + name string + opts []Option +}{ + {name: "fd"}, + {name: "mmap", opts: []Option{WithMmap()}}, +} + +func TestBoundOSConcurrentReadAt(t *testing.T) { + t.Parallel() + + for _, m := range modes { + t.Run(m.name, func(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + want := writePattern(t, filepath.Join(dir, "data"), 64*1024) + + fs := newTestBoundOS(t, dir, m.opts...) + f, err := fs.Open("data") + require.NoError(t, err) + t.Cleanup(func() { _ = f.Close() }) + + runConcurrentReadAt(t, f, want) + }) + } +} + +func TestRootOSConcurrentReadAt(t *testing.T) { + t.Parallel() + + for _, m := range modes { + t.Run(m.name, func(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + want := writePattern(t, filepath.Join(dir, "data"), 64*1024) + + root, err := os.OpenRoot(dir) + require.NoError(t, err) + t.Cleanup(func() { _ = root.Close() }) + + fs, err := FromRoot(root, m.opts...) + require.NoError(t, err) + + f, err := fs.Open("data") + require.NoError(t, err) + t.Cleanup(func() { _ = f.Close() }) + + runConcurrentReadAt(t, f, want) + }) + } +} + +func TestOpenCloseSemantics(t *testing.T) { + t.Parallel() + + for _, m := range modes { + t.Run(m.name, func(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + writePattern(t, filepath.Join(dir, "data"), 4096) + + fs := newTestBoundOS(t, dir, m.opts...) + f, err := fs.Open("data") + require.NoError(t, err) + + buf := make([]byte, 16) + _, err = f.ReadAt(buf, 0) + require.NoError(t, err) + + require.NoError(t, f.Close()) + _, err = f.ReadAt(buf, 0) + require.ErrorIs(t, err, os.ErrClosed) + }) + } +} + +// TestOpenEmptyFile pins the empty-file path on both backings. With +// WithMmap on darwin/linux the mmap path's zero-size guard falls back +// to the regular *file wrapper; without it (or off-platform) it goes +// to *file directly. Both must return (0, io.EOF) at offset 0. +func TestOpenEmptyFile(t *testing.T) { + t.Parallel() + + for _, m := range modes { + t.Run(m.name, func(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "empty"), nil, 0o600)) + + fs := newTestBoundOS(t, dir, m.opts...) + f, err := fs.Open("empty") + require.NoError(t, err) + t.Cleanup(func() { _ = f.Close() }) + + buf := make([]byte, 8) + n, err := f.ReadAt(buf, 0) + require.Equal(t, 0, n) + require.ErrorIs(t, err, io.EOF) + }) + } +} + +// TestOpenReadSeekCursor exercises the mmap-backed file's Read+Seek +// cursor against the regular *file's equivalent semantics. Both +// backings must advance the cursor on Read, support SeekStart, +// SeekCurrent, and SeekEnd, and return io.EOF when read past the end. +func TestOpenReadSeekCursor(t *testing.T) { + t.Parallel() + + for _, m := range modes { + t.Run(m.name, func(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + want := writePattern(t, filepath.Join(dir, "data"), 1024) + + fs := newTestBoundOS(t, dir, m.opts...) + f, err := fs.Open("data") + require.NoError(t, err) + t.Cleanup(func() { _ = f.Close() }) + + // Sequential read advances cursor. + head := make([]byte, 32) + n, err := f.Read(head) + require.NoError(t, err) + require.Equal(t, 32, n) + require.Equal(t, want[:32], head) + + // Continued read picks up where the cursor left off. + next := make([]byte, 32) + n, err = f.Read(next) + require.NoError(t, err) + require.Equal(t, 32, n) + require.Equal(t, want[32:64], next) + + // SeekStart rewinds. + pos, err := f.Seek(0, io.SeekStart) + require.NoError(t, err) + require.Equal(t, int64(0), pos) + again := make([]byte, 32) + _, err = f.Read(again) + require.NoError(t, err) + require.Equal(t, want[:32], again) + + // SeekEnd lands at len(data); Read returns EOF. + pos, err = f.Seek(0, io.SeekEnd) + require.NoError(t, err) + require.Equal(t, int64(1024), pos) + tail := make([]byte, 32) + n, err = f.Read(tail) + require.Equal(t, 0, n) + require.ErrorIs(t, err, io.EOF) + }) + } +} + +// TestOpenReadOnlyMmapErrorsOnWrite asserts the mmap-backed file +// rejects writes. On platforms where mmap is unavailable the test +// row falls back to *file, whose writes return the os.File-level +// error for an O_RDONLY handle (also a write rejection). Either way +// Write must return an error. +func TestOpenReadOnlyMmapErrorsOnWrite(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + writePattern(t, filepath.Join(dir, "data"), 256) + + fs := newTestBoundOS(t, dir, WithMmap()) + f, err := fs.Open("data") + require.NoError(t, err) + t.Cleanup(func() { _ = f.Close() }) + + _, err = f.Write([]byte("nope")) + require.Error(t, err) + + _, err = f.WriteAt([]byte("nope"), 0) + require.Error(t, err) + + require.Error(t, f.Truncate(0)) +} + +// TestOpenStatNameAcrossModes pins f.Stat().Name() to the basename of +// the opened path on both backings. *file inherits this from +// *os.File.Stat(); *mmapFile delegates to the same. Without this +// pin, a future change to either wrapper could surface the full +// display path through Stat and diverge silently between modes. +func TestOpenStatNameAcrossModes(t *testing.T) { + t.Parallel() + + for _, m := range modes { + t.Run(m.name, func(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(dir, "sub"), 0o755)) + writePattern(t, filepath.Join(dir, "sub", "data"), 256) + + fs := newTestBoundOS(t, dir, m.opts...) + f, err := fs.Open("sub/data") + require.NoError(t, err) + t.Cleanup(func() { _ = f.Close() }) + + info, err := f.Stat() + require.NoError(t, err) + require.Equal(t, "data", info.Name()) + }) + } +} + +// TestOpenZeroLengthReadAtEOF pins that a Read/ReadAt with an empty +// buffer at EOF returns (0, nil) rather than (0, io.EOF), matching +// the fd backing's *os.File.Read behaviour. mmap's natural +// implementation (a bytes-style EOF check before the empty-buffer +// fast path) would otherwise diverge. +func TestOpenZeroLengthReadAtEOF(t *testing.T) { + t.Parallel() + + for _, m := range modes { + t.Run(m.name, func(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + writePattern(t, filepath.Join(dir, "data"), 8) + + fs := newTestBoundOS(t, dir, m.opts...) + f, err := fs.Open("data") + require.NoError(t, err) + t.Cleanup(func() { _ = f.Close() }) + + // Advance the cursor to EOF, then issue a zero-length Read. + _, err = f.Seek(0, io.SeekEnd) + require.NoError(t, err) + n, err := f.Read(nil) + require.Equal(t, 0, n) + require.NoError(t, err) + + // Same for ReadAt at an EOF-or-past offset. + n, err = f.ReadAt(nil, 8) + require.Equal(t, 0, n) + require.NoError(t, err) + }) + } +} + +// TestOpenConcurrentReadCursorSafety asserts that concurrent Read +// calls on the same handle don't race on the cursor. Read is +// serialised internally; this test exists so a future change that +// drops the lock surfaces under -race. +func TestOpenConcurrentReadCursorSafety(t *testing.T) { + t.Parallel() + + for _, m := range modes { + t.Run(m.name, func(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + writePattern(t, filepath.Join(dir, "data"), 64*1024) + + fs := newTestBoundOS(t, dir, m.opts...) + f, err := fs.Open("data") + require.NoError(t, err) + t.Cleanup(func() { _ = f.Close() }) + + const workers = 8 + const iters = 50 + var wg sync.WaitGroup + wg.Add(workers) + for range workers { + go func() { + defer wg.Done() + buf := make([]byte, 256) + for range iters { + _, _ = f.Read(buf) + } + }() + } + wg.Wait() + }) + } +} + +// TestOpenBackingSelection asserts that WithMmap actually flips the +// concrete backing on darwin/linux. On other platforms both modes +// return *file because mmap is unavailable. +func TestOpenBackingSelection(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + writePattern(t, filepath.Join(dir, "data"), 4096) + + t.Run("default-is-fd", func(t *testing.T) { + t.Parallel() + fs := newTestBoundOS(t, dir) + f, err := fs.Open("data") + require.NoError(t, err) + t.Cleanup(func() { _ = f.Close() }) + _, ok := f.(*file) + require.True(t, ok, "default backing should be *file, got %T", f) + }) + + t.Run("with-mmap-uses-mmap-when-available", func(t *testing.T) { + t.Parallel() + fs := newTestBoundOS(t, dir, WithMmap()) + f, err := fs.Open("data") + require.NoError(t, err) + t.Cleanup(func() { _ = f.Close() }) + assertMmapBackingWhenAvailable(t, f) + }) + + t.Run("with-mmap-write-mode-still-uses-fd", func(t *testing.T) { + t.Parallel() + fs := newTestBoundOS(t, dir, WithMmap()) + f, err := fs.OpenFile("data", os.O_RDWR, 0) + require.NoError(t, err) + t.Cleanup(func() { _ = f.Close() }) + _, ok := f.(*file) + require.True(t, ok, "write-mode opens should bypass mmap, got %T", f) + }) +} + +// Ensure *file satisfies billy.File at compile-time so type +// assertions in tests are guaranteed valid. +var _ billy.File = (*file)(nil) diff --git a/osfs/os.go b/osfs/os.go index 26263a2..1f87088 100644 --- a/osfs/os.go +++ b/osfs/os.go @@ -4,6 +4,7 @@ package osfs import ( + "errors" "io" "io/fs" "os" @@ -18,7 +19,7 @@ const ( ) // Default Filesystem representing the root of the os filesystem. -var Default = newBoundOS(string(os.PathSeparator)) +var Default billy.Filesystem = newBoundOS(string(os.PathSeparator)) // New returns a new OS filesystem rooted at baseDir. // @@ -26,15 +27,18 @@ var Default = newBoundOS(string(os.PathSeparator)) // via [os.Root], opened and closed per operation. For better performance // with caller-managed lifecycle, use [FromRoot] instead. // -// All [Option] values are accepted for API compatibility but have no -// effect on the returned implementation. +// [WithMmap] enables an mmap-backed implementation of [BoundOS.Open] on +// supported platforms; all other [Option] values are accepted for API +// compatibility but have no effect on the returned implementation. func New(baseDir string, opts ...Option) billy.Filesystem { o := &options{} for _, opt := range opts { opt(o) } - return newBoundOS(baseDir) + fs := newBoundOS(baseDir) + fs.mmap = o.mmap + return fs } // WithBoundOS selects the [BoundOS] implementation. @@ -49,8 +53,16 @@ func WithBoundOS() Option { type options struct { Type + mmap bool } +// errMmapUnavailable is returned by newMmapFile when the file cannot +// be memory-mapped for benign reasons (zero size, size beyond +// platform int, mmap rejected by the kernel, platform without mmap +// support). Callers in osfs treat this as a fall-through signal and +// return the regular *file wrapper instead. +var errMmapUnavailable = errors.New("osfs: mmap unavailable for this file") + // Type identifies an osfs implementation. type Type int diff --git a/osfs/os_bound.go b/osfs/os_bound.go index 6a3d11c..7b5281f 100644 --- a/osfs/os_bound.go +++ b/osfs/os_bound.go @@ -46,9 +46,10 @@ import ( // in [ErrPathEscapesParent]. type BoundOS struct { baseDir string + mmap bool } -func newBoundOS(d string) billy.Filesystem { +func newBoundOS(d string) *BoundOS { if d == "" { d = string(os.PathSeparator) } @@ -56,6 +57,17 @@ func newBoundOS(d string) billy.Filesystem { return &BoundOS{baseDir: d} } +// clone returns a new [BoundOS] rooted at d that carries over fs's +// configurable behaviour (currently just the [WithMmap] flag). Used +// to construct sub-filesystems via [BoundOS.Chroot] without silently +// dropping caller-supplied options. +func (fs *BoundOS) clone(d string) *BoundOS { + if d == "" { + d = string(os.PathSeparator) + } + return &BoundOS{baseDir: hostPath(d), mmap: fs.mmap} +} + // rootFS opens a temporary [RootOS] and returns a cleanup function that // closes the underlying [os.Root]. func (fs *BoundOS) rootFS() (*RootOS, func(), error) { @@ -68,7 +80,7 @@ func (fs *BoundOS) rootFSWithCreate(name string, createBase bool) (*RootOS, func if err != nil { return nil, func() {}, err } - return &RootOS{root: r}, func() { r.Close() }, nil + return &RootOS{root: r, mmap: fs.mmap}, func() { r.Close() }, nil } func (fs *BoundOS) Capabilities() billy.Capability { @@ -204,13 +216,13 @@ func (fs *BoundOS) Chmod(path string, mode gofs.FileMode) error { // result of joining the provided path with the underlying base dir. func (fs *BoundOS) Chroot(path string) (billy.Filesystem, error) { if hostPath, ok := fs.hostAbsolutePath(path); ok { - return newBoundOS(hostPath), nil + return fs.clone(hostPath), nil } rfs, cleanup, err := fs.rootFS() if err != nil { if errors.Is(err, os.ErrNotExist) { - return newBoundOS(fs.chrootPath(path)), nil + return fs.clone(fs.chrootPath(path)), nil } return nil, err } @@ -223,13 +235,13 @@ func (fs *BoundOS) Chroot(path string) (billy.Filesystem, error) { childRoot, err := rfs.root.OpenRoot(rel) if err != nil { if errors.Is(err, os.ErrNotExist) { - return newBoundOS(fs.chrootPath(path)), nil + return fs.clone(fs.chrootPath(path)), nil } return nil, err } defer childRoot.Close() - return newBoundOS(filepath.Clean(childRoot.Name())), nil + return fs.clone(filepath.Clean(childRoot.Name())), nil } // Root returns the current base dir of the billy.Filesystem. diff --git a/osfs/os_bound_test.go b/osfs/os_bound_test.go index ca58b5d..7a4e526 100644 --- a/osfs/os_bound_test.go +++ b/osfs/os_bound_test.go @@ -36,10 +36,8 @@ import ( func TestBoundOSCapabilities(t *testing.T) { dir := t.TempDir() fs := newBoundOS(dir) - c, ok := fs.(billy.Capable) - require.True(t, ok) - assert.Equal(t, boundCapabilities(), c.Capabilities()) + assert.Equal(t, boundCapabilities(), fs.Capabilities()) } func TestFromRoot(t *testing.T) { @@ -209,7 +207,7 @@ func TestOpen(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { dir := t.TempDir() - fs := newBoundOS(dir) + var fs billy.Filesystem = newBoundOS(dir) if tt.before != nil { fs = tt.before(t, dir) } @@ -517,7 +515,7 @@ func TestRemove(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { dir := t.TempDir() - fs := newBoundOS(dir) + var fs billy.Filesystem = newBoundOS(dir) if tt.before != nil { fs = tt.before(t, dir) } @@ -739,10 +737,13 @@ func requireFileContents(t *testing.T, filename string, want []byte) { assert.Equal(t, want, got) } -func newTestBoundOS(t *testing.T, dir string) *BoundOS { +func newTestBoundOS(t *testing.T, dir string, opts ...Option) *BoundOS { t.Helper() - fs, ok := newBoundOS(dir).(*BoundOS) + if len(opts) == 0 { + return newBoundOS(dir) + } + fs, ok := New(dir, opts...).(*BoundOS) require.True(t, ok) return fs } diff --git a/osfs/os_js.go b/osfs/os_js.go index 0a65286..9b80c32 100644 --- a/osfs/os_js.go +++ b/osfs/os_js.go @@ -87,10 +87,20 @@ func WithBoundOS() Option { } } +// WithMmap is a no-op on js/wasm; the in-memory backing has no fd to +// map. The function exists so cross-platform callers can pass the +// option without build tags. +func WithMmap() Option { + return func(o *options) { + o.mmap = true + } +} + type Option func(*options) type options struct { Type + mmap bool } type Type int diff --git a/osfs/os_options.go b/osfs/os_options.go index 72fbfb0..75b9606 100644 --- a/osfs/os_options.go +++ b/osfs/os_options.go @@ -3,3 +3,37 @@ package osfs type Option func(*options) + +// WithMmap opts the filesystem returned by [New] (and any [RootOS] +// returned by [FromRoot]) into an mmap-backed implementation of +// [billy.File] for read-only opens on platforms that support it +// (currently darwin and linux). On other platforms the option is +// accepted but has no effect. +// +// EXPERIMENTAL: this option's semantics may change. Today every +// read-only [BoundOS.Open] / [RootOS.Open] (and any [BoundOS.OpenFile] +// / [RootOS.OpenFile] without write flags) returns a mmap-backed +// handle when the option is set. That includes small files and +// sequential-read workloads where mmap can be neutral or net- +// negative compared to plain file I/O. If we find a meaningful +// reason to make the opt-in finer-grained (per-call rather than +// per-FS), the option's effect on [Open]/[OpenFile] will narrow +// accordingly. +// +// mmap also changes failure semantics: truncating the underlying +// file while a read is in flight raises SIGBUS instead of returning +// an error, and replacing the file via rename leaves the mapping +// pointing at the old inode. Callers that may see the file mutate +// underneath them should leave the option off. +// +// The mmap-backed file is read-only by construction; it does not +// satisfy [billy.Syncer] (Sync is meaningless on a read-only +// mapping) or the [Locker] interface even though the surrounding +// filesystem advertises both capabilities. Code that type-asserts +// against those interfaces on a handle returned by an open with +// [WithMmap] active should be prepared for the assertion to fail. +func WithMmap() Option { + return func(o *options) { + o.mmap = true + } +} diff --git a/osfs/os_rootfs.go b/osfs/os_rootfs.go index db3cfc5..0f4f1e1 100644 --- a/osfs/os_rootfs.go +++ b/osfs/os_rootfs.go @@ -18,11 +18,20 @@ import ( // FromRoot creates a new [RootOS] from an [os.Root]. // The provided root is used directly for all operations and the caller // is responsible for its lifecycle. Root must not be nil. -func FromRoot(root *os.Root) (*RootOS, error) { +// +// [WithMmap] enables an mmap-backed [billy.File] from [RootOS.Open] +// (and [RootOS.OpenFile] when opened read-only) on supported +// platforms; all other [Option] values are accepted for API +// compatibility but have no effect. +func FromRoot(root *os.Root, opts ...Option) (*RootOS, error) { if root == nil { return nil, errors.New("root must not be nil") } - return &RootOS{root: root}, nil + o := &options{} + for _, opt := range opts { + opt(o) + } + return &RootOS{root: root, mmap: o.mmap}, nil } // RootOS is a high-performance fs implementation based on a caller-managed @@ -44,6 +53,7 @@ func FromRoot(root *os.Root) (*RootOS, error) { // in [ErrPathEscapesParent]. type RootOS struct { root *os.Root + mmap bool } func (fs *RootOS) Capabilities() billy.Capability { @@ -70,7 +80,7 @@ func (fs *RootOS) OpenFile(name string, flag int, perm gofs.FileMode) (billy.Fil } f, err := fs.root.OpenFile(openName, flag, perm) if err == nil { - return &file{File: f, name: display}, nil + return fs.wrapOpenedFile(f, display, flag) } // os.Root rejects an absolute symlink target even when it points back @@ -83,7 +93,7 @@ func (fs *RootOS) OpenFile(name string, flag int, perm gofs.FileMode) (billy.Fil if target, readlinkErr := fs.root.Readlink(openName); readlinkErr == nil { if targetRel, ok := fs.absoluteSymlinkTarget(target); ok { if f, err = fs.root.OpenFile(targetRel, flag, perm); err == nil { - return &file{File: f, name: display}, nil + return fs.wrapOpenedFile(f, display, flag) } } } @@ -94,6 +104,28 @@ func (fs *RootOS) OpenFile(name string, flag int, perm gofs.FileMode) (billy.Fil return nil, translateError(err, rel) } +// wrapOpenedFile picks the [billy.File] shape for f based on the +// filesystem's [WithMmap] configuration and the open flags. For +// read-only opens on [WithMmap]-configured filesystems on supported +// platforms it returns a [*mmapFile]; in every other case (write +// access, mmap disabled, platforms without mmap support, or mmap +// unavailable for this specific file) it returns the regular [*file] +// wrapper around the underlying [*os.File]. +func (fs *RootOS) wrapOpenedFile(f *os.File, name string, flag int) (billy.File, error) { + if fs.mmap && flag&(os.O_WRONLY|os.O_RDWR) == 0 { + mf, err := newMmapFile(f, name) + if err == nil { + return mf, nil + } + if !errors.Is(err, errMmapUnavailable) { + // Real failure (e.g. stat); newMmapFile already closed f. + return nil, err + } + // Fall through to the regular wrapper. + } + return &file{File: f, name: name}, nil +} + func (fs *RootOS) ReadDir(name string) ([]gofs.DirEntry, error) { rel := fs.toRelative(name) if rel == "" {