client-nowasm_test.go | 35 +++++++++++++++++++++++------------ client.go | 5 +++-- cmd/torrent-verify/main.go | 18 ++++++++++++++---- mmap_span/mmap_span.go | 16 ++++++++++------ storage/file_test.go | 1 + storage/issue95_test.go | 10 ++++++++-- storage/issue96_test.go | 4 +++- storage/mmap.go | 81 ++++++++++++++++++++++++++++++++++++++++------------- storage/mmap_test.go | 25 +++++++++++++++++++++++++ t.go | 5 ++++- diff --git a/client-nowasm_test.go b/client-nowasm_test.go index 41645b6ee8acf42be58a2214b2fb8eefa6be302e..3ce43f2ab27c268aa9f4a1fc3a65c767e3588e51 100644 --- a/client-nowasm_test.go +++ b/client-nowasm_test.go @@ -7,7 +7,8 @@ import ( "os" "testing" - "github.com/stretchr/testify/assert" + qt "github.com/frankban/quicktest" + "github.com/stretchr/testify/require" "github.com/anacrolix/torrent/internal/testutil" @@ -38,24 +39,34 @@ if err != nil { t.Fatalf("removing torrent dummy data dir: %v", err) } }() + logErr := func(f func() error, msg string) { + err := f() + t.Logf("%s: %v", msg, err) + if err != nil { + t.Fail() + } + } cfg := TestingConfig(t) cfg.Seed = false cfg.Debug = true cfg.DataDir = dir comp, err := storage.NewBoltPieceCompletion(dir) - require.NoError(t, err) - defer comp.Close() - cfg.DefaultStorage = storage.NewMMapWithCompletion(dir, comp) + c := qt.New(t) + c.Assert(err, qt.IsNil) + defer logErr(comp.Close, "closing bolt piece completion") + mmapStorage := storage.NewMMapWithCompletion(dir, comp) + defer logErr(mmapStorage.Close, "closing mmap storage") + cfg.DefaultStorage = mmapStorage cl, err := NewClient(cfg) - require.NoError(t, err) - defer cl.Close() + c.Assert(err, qt.IsNil) + defer logErr(cl.Close, "closing client") tor, new, err := cl.AddTorrentSpec(TorrentSpecFromMetaInfo(mi)) - require.NoError(t, err) - assert.True(t, new) - require.True(t, cl.WaitAll()) + c.Assert(err, qt.IsNil) + c.Assert(new, qt.IsTrue) + c.Assert(cl.WaitAll(), qt.IsTrue) tor.Drop() _, new, err = cl.AddTorrentSpec(TorrentSpecFromMetaInfo(mi)) - require.NoError(t, err) - assert.True(t, new) - require.True(t, cl.WaitAll()) + c.Assert(err, qt.IsNil) + c.Assert(new, qt.IsTrue) + c.Assert(cl.WaitAll(), qt.IsTrue) } diff --git a/client.go b/client.go index a87e44391f657508406c243b74df06602c81b030..f1a7a3c73f117100774b9d2c0239becff1689991 100644 --- a/client.go +++ b/client.go @@ -425,9 +425,10 @@ } } // Stops the client. All connections to peers are closed and all activity will come to a halt. -func (cl *Client) Close() (errs []error) { +func (cl *Client) Close() error { var closeGroup sync.WaitGroup // For concurrent cleanup to complete before returning cl.lock() + var errs []error for _, t := range cl.torrents { err := t.close(&closeGroup) if err != nil { @@ -441,7 +442,7 @@ cl.closed.Set() cl.unlock() cl.event.Broadcast() closeGroup.Wait() // defer is LIFO. We want to Wait() after cl.unlock() - return + return errors.Join(errs...) } func (cl *Client) ipBlockRange(ip net.IP) (r iplist.Range, blocked bool) { diff --git a/cmd/torrent-verify/main.go b/cmd/torrent-verify/main.go index e945f12f6db82e6cca80cef9f54a48950d9f5e50..ae89b5f19a4b311798ec5fcdcff6204c89688040 100644 --- a/cmd/torrent-verify/main.go +++ b/cmd/torrent-verify/main.go @@ -9,6 +9,8 @@ "log" "os" "path/filepath" + "github.com/anacrolix/torrent/storage" + "github.com/anacrolix/tagflag" "github.com/edsrzf/mmap-go" @@ -16,12 +18,16 @@ "github.com/anacrolix/torrent/metainfo" "github.com/anacrolix/torrent/mmap_span" ) -func mmapFile(name string) (mm mmap.MMap, err error) { +func mmapFile(name string) (mm storage.FileMapping, err error) { f, err := os.Open(name) if err != nil { return } - defer f.Close() + defer func() { + if err != nil { + f.Close() + } + }() fi, err := f.Stat() if err != nil { return @@ -29,7 +35,11 @@ } if fi.Size() == 0 { return } - return mmap.MapRegion(f, -1, mmap.RDONLY, mmap.COPY, 0) + reg, err := mmap.MapRegion(f, -1, mmap.RDONLY, mmap.COPY, 0) + if err != nil { + return + } + return storage.WrapFileMapping(reg, f), nil } func verifyTorrent(info *metainfo.Info, root string) error { @@ -40,7 +50,7 @@ mm, err := mmapFile(filename) if err != nil { return err } - if int64(len(mm)) != file.Length { + if int64(len(mm.Bytes())) != file.Length { return fmt.Errorf("file %q has wrong length", filename) } span.Append(mm) diff --git a/mmap_span/mmap_span.go b/mmap_span/mmap_span.go index de74956bb8287ea59e9e28bd953f62dd06804fd8..22c394f76da92870cfad3a1eb9dee620a61f5e70 100644 --- a/mmap_span/mmap_span.go +++ b/mmap_span/mmap_span.go @@ -5,18 +5,22 @@ "fmt" "io" "sync" - "github.com/edsrzf/mmap-go" - "github.com/anacrolix/torrent/segments" ) + +type Mmap interface { + Flush() error + Unmap() error + Bytes() []byte +} type MMapSpan struct { mu sync.RWMutex - mMaps []mmap.MMap + mMaps []Mmap segmentLocater segments.Index } -func (ms *MMapSpan) Append(mMap mmap.MMap) { +func (ms *MMapSpan) Append(mMap Mmap) { ms.mMaps = append(ms.mMaps, mMap) } @@ -53,7 +57,7 @@ me.segmentLocater = segments.NewIndex(func() (segments.Length, bool) { if i == len(me.mMaps) { return -1, false } - l := int64(len(me.mMaps[i])) + l := int64(len(me.mMaps[i].Bytes())) i++ return l, true }) @@ -77,7 +81,7 @@ } func (ms *MMapSpan) locateCopy(copyArgs func(remainingArgument, mmapped []byte) (dst, src []byte), p []byte, off int64) (n int) { ms.segmentLocater.Locate(segments.Extent{off, int64(len(p))}, func(i int, e segments.Extent) bool { - mMapBytes := ms.mMaps[i][e.Start:] + mMapBytes := ms.mMaps[i].Bytes()[e.Start:] // log.Printf("got segment %v: %v, copying %v, %v", i, e, len(p), len(mMapBytes)) _n := copyBytes(copyArgs(p, mMapBytes)) p = p[_n:] diff --git a/storage/file_test.go b/storage/file_test.go index 604db70144c53beeb80ec9321787e4a31efca6bb..a6c69fa28dfef0ac12a6095d582c4ca58f1355da 100644 --- a/storage/file_test.go +++ b/storage/file_test.go @@ -17,6 +17,7 @@ func TestShortFile(t *testing.T) { td := t.TempDir() s := NewFile(td) + defer s.Close() info := &metainfo.Info{ Name: "a", Length: 2, diff --git a/storage/issue95_test.go b/storage/issue95_test.go index bda208699fe1df032212a9eb6c29ea8c9be0c229..92370797b6698248458d934aea6a6e498ceb04d6 100644 --- a/storage/issue95_test.go +++ b/storage/issue95_test.go @@ -19,12 +19,14 @@ Pieces: make([]byte, 20), } t1, err := c.OpenTorrent(i1, metainfo.HashBytes([]byte("a"))) require.NoError(t, err) + defer t1.Close() i2 := &metainfo.Info{ Files: []metainfo.FileInfo{{Path: []string{"a"}}}, Pieces: make([]byte, 20), } t2, err := c.OpenTorrent(i2, metainfo.HashBytes([]byte("b"))) require.NoError(t, err) + defer t2.Close() t2p := t2.Piece(i2.Piece(0)) assert.NoError(t, t1.Close()) assert.NotPanics(t, func() { t2p.Completion() }) @@ -32,12 +34,16 @@ } func TestIssue95File(t *testing.T) { td := t.TempDir() - testIssue95(t, NewFile(td)) + cs := NewFile(td) + defer cs.Close() + testIssue95(t, cs) } func TestIssue95MMap(t *testing.T) { td := t.TempDir() - testIssue95(t, NewMMap(td)) + cs := NewMMap(td) + defer cs.Close() + testIssue95(t, cs) } func TestIssue95ResourcePieces(t *testing.T) { diff --git a/storage/issue96_test.go b/storage/issue96_test.go index d06b302b5039b5c653371678776e015170b2b74a..726c11c803148e80bffa2df3166e605505bf3b0b 100644 --- a/storage/issue96_test.go +++ b/storage/issue96_test.go @@ -10,7 +10,9 @@ ) func testMarkedCompleteMissingOnRead(t *testing.T, csf func(string) ClientImplCloser) { td := t.TempDir() - cs := NewClient(csf(td)) + cic := csf(td) + defer cic.Close() + cs := NewClient(cic) info := &metainfo.Info{ PieceLength: 1, Files: []metainfo.FileInfo{{Path: []string{"a"}, Length: 1}}, diff --git a/storage/mmap.go b/storage/mmap.go index 00b2721c1199767d5f9982d2f243820a6fb325a4..1b5d05a24390bff05afeaabfda38d432a6097fff 100644 --- a/storage/mmap.go +++ b/storage/mmap.go @@ -124,21 +124,19 @@ if err != nil { return } fileName := filepath.Join(location, safeName) - var mm mmap.MMap + var mm FileMapping mm, err = mmapFile(fileName, miFile.Length) if err != nil { err = fmt.Errorf("file %q: %s", miFile.DisplayPath(md), err) return } - if mm != nil { - mms.Append(mm) - } + mms.Append(mm) } mms.InitIndex() return } -func mmapFile(name string, size int64) (ret mmap.MMap, err error) { +func mmapFile(name string, size int64) (_ FileMapping, err error) { dir := filepath.Dir(name) err = os.MkdirAll(dir, 0o750) if err != nil { @@ -150,7 +148,11 @@ file, err = os.OpenFile(name, os.O_CREATE|os.O_RDWR, 0o666) if err != nil { return } - defer file.Close() + defer func() { + if err != nil { + file.Close() + } + }() var fi os.FileInfo fi, err = file.Stat() if err != nil { @@ -164,22 +166,61 @@ if err != nil { return } } - if size == 0 { - // Can't mmap() regions with length 0. + return func() (ret mmapWithFile, err error) { + ret.f = file + if size == 0 { + // Can't mmap() regions with length 0. + return + } + intLen := int(size) + if int64(intLen) != size { + err = errors.New("size too large for system") + return + } + ret.mmap, err = mmap.MapRegion(file, intLen, mmap.RDWR, 0, 0) + if err != nil { + err = fmt.Errorf("error mapping region: %s", err) + return + } + if int64(len(ret.mmap)) != size { + panic(len(ret.mmap)) + } return - } - intLen := int(size) - if int64(intLen) != size { - err = errors.New("size too large for system") - return + }() +} + +// Combines a mmapped region and file into a storage Mmap abstraction, which handles closing the +// mmap file handle. +func WrapFileMapping(region mmap.MMap, file *os.File) FileMapping { + return mmapWithFile{ + f: file, + mmap: region, } - ret, err = mmap.MapRegion(file, intLen, mmap.RDWR, 0, 0) - if err != nil { - err = fmt.Errorf("error mapping region: %s", err) - return +} + +type FileMapping = mmap_span.Mmap + +// Handles closing the mmap's file handle (needed for Windows). Could be implemented differently by +// OS. +type mmapWithFile struct { + f *os.File + mmap mmap.MMap +} + +func (m mmapWithFile) Flush() error { + return m.mmap.Flush() +} + +func (m mmapWithFile) Unmap() (err error) { + if m.mmap != nil { + err = m.mmap.Unmap() } - if int64(len(ret)) != size { - panic(len(ret)) + return errors.Join(err, m.f.Close()) +} + +func (m mmapWithFile) Bytes() []byte { + if m.mmap == nil { + return nil } - return + return m.mmap } diff --git a/storage/mmap_test.go b/storage/mmap_test.go new file mode 100644 index 0000000000000000000000000000000000000000..54260ecb5cb8ea11856518eece3dec458a2f9531 --- /dev/null +++ b/storage/mmap_test.go @@ -0,0 +1,25 @@ +package storage + +import ( + "testing" + + qt "github.com/frankban/quicktest" + + "github.com/anacrolix/torrent/internal/testutil" +) + +func TestMmapWindows(t *testing.T) { + c := qt.New(t) + dir, mi := testutil.GreetingTestTorrent() + cs := NewMMap(dir) + defer func() { + c.Check(cs.Close(), qt.IsNil) + }() + info, err := mi.UnmarshalInfo() + c.Assert(err, qt.IsNil) + ts, err := cs.OpenTorrent(&info, mi.HashInfoBytes()) + c.Assert(err, qt.IsNil) + defer func() { + c.Check(ts.Close(), qt.IsNil) + }() +} diff --git a/t.go b/t.go index 765f3cf2a66ee6688b3895a6bd428c35e5bf9945..af4fdec7f3c000cc6ab2fd36cb5204ac891c403a 100644 --- a/t.go +++ b/t.go @@ -100,7 +100,10 @@ var wg sync.WaitGroup defer wg.Wait() t.cl.lock() defer t.cl.unlock() - t.cl.dropTorrent(t.infoHash, &wg) + err := t.cl.dropTorrent(t.infoHash, &wg) + if err != nil { + panic(err) + } } // Number of bytes of the entire torrent we have completed. This is the sum of