client.go | 7 ++++--- client_test.go | 4 ++-- config.go | 2 +- storage/boltdb.go | 2 +- storage/file.go | 6 +++--- storage/interface.go | 6 +++++- storage/issue96_test.go | 2 +- storage/mmap.go | 4 ++-- storage/piece_resource.go | 2 +- t.go | 4 ++-- test/transfer_test.go | 23 +++++++++++++---------- diff --git a/client.go b/client.go index 3a3084a4ab71d59d921ce3952a610f7532fc91ff..2ce57d0fb82207f2fdc5f560fa21249fd1b495ec 100644 --- a/client.go +++ b/client.go @@ -190,13 +190,14 @@ cl.extensionBytes = defaultPeerExtensionBytes() cl.event.L = cl.locker() storageImpl := cfg.DefaultStorage if storageImpl == nil { - // We'd use mmap but HFS+ doesn't support sparse files. - storageImpl = storage.NewFile(cfg.DataDir) + // We'd use mmap by default but HFS+ doesn't support sparse files. + storageImplCloser := storage.NewFile(cfg.DataDir) cl.onClose = append(cl.onClose, func() { - if err := storageImpl.Close(); err != nil { + if err := storageImplCloser.Close(); err != nil { cl.logger.Printf("error closing default storage: %s", err) } }) + storageImpl = storageImplCloser } cl.defaultStorage = storage.NewClient(storageImpl) if cfg.IPBlocklist != nil { diff --git a/client_test.go b/client_test.go index 0f65da6cc557517a22e53a18d955b38f0dcc647b..0ca330a93f924bee300c7ee3d7a312f1cac664e2 100644 --- a/client_test.go +++ b/client_test.go @@ -151,7 +151,7 @@ defer tt.Drop() } } -func fileCachePieceResourceStorage(fc *filecache.Cache) storage.ClientImpl { +func fileCachePieceResourceStorage(fc *filecache.Cache) storage.ClientImplCloser { return storage.NewResourcePieces(fc.AsResourceProvider()) } @@ -366,7 +366,7 @@ ts.Piece(p).WriteAt(b[p.Offset():p.Offset()+p.Length()], 0) } } -func testAddTorrentPriorPieceCompletion(t *testing.T, alreadyCompleted bool, csf func(*filecache.Cache) storage.ClientImpl) { +func testAddTorrentPriorPieceCompletion(t *testing.T, alreadyCompleted bool, csf func(*filecache.Cache) storage.ClientImplCloser) { fileCacheDir, err := ioutil.TempDir("", "") require.NoError(t, err) defer os.RemoveAll(fileCacheDir) diff --git a/config.go b/config.go index ee41ba0248586bd61393cdff46aadb828a4d4ee3..c0ea9ed373116688acac581a09ee6b30824ba822 100644 --- a/config.go +++ b/config.go @@ -65,7 +65,7 @@ // For the bittorrent protocol. DisableTCP bool `long:"disable-tcp"` // Called to instantiate storage for each added torrent. Builtin backends // are in the storage package. If not set, the "file" implementation is - // used. + // used (and Closed when the Client is Closed). DefaultStorage storage.ClientImpl HeaderObfuscationPolicy HeaderObfuscationPolicy diff --git a/storage/boltdb.go b/storage/boltdb.go index 045bb60052807dfc8119db855bae0256565a3180..6e4faddc2cc69bc8206b236a955bac6701e20af9 100644 --- a/storage/boltdb.go +++ b/storage/boltdb.go @@ -26,7 +26,7 @@ cl *boltDBClient ih metainfo.Hash } -func NewBoltDB(filePath string) ClientImpl { +func NewBoltDB(filePath string) ClientImplCloser { db, err := bolt.Open(filepath.Join(filePath, "bolt.db"), 0600, &bolt.Options{ Timeout: time.Second, }) diff --git a/storage/file.go b/storage/file.go index fd4d56014f6cb230529ddecc82e26108411eac7f..415ac5df49f19a48b6c2d700eb8e5da66b9a216d 100644 --- a/storage/file.go +++ b/storage/file.go @@ -28,11 +28,11 @@ return filepath.Join(baseDir, infoHash.HexString()) } // All Torrent data stored in this baseDir -func NewFile(baseDir string) ClientImpl { +func NewFile(baseDir string) ClientImplCloser { return NewFileWithCompletion(baseDir, pieceCompletionForDir(baseDir)) } -func NewFileWithCompletion(baseDir string, completion PieceCompletion) ClientImpl { +func NewFileWithCompletion(baseDir string, completion PieceCompletion) *fileClientImpl { return newFileWithCustomPathMakerAndCompletion(baseDir, nil, completion) } @@ -46,7 +46,7 @@ func NewFileWithCustomPathMaker(baseDir string, pathMaker func(baseDir string, info *metainfo.Info, infoHash metainfo.Hash) string) ClientImpl { return newFileWithCustomPathMakerAndCompletion(baseDir, pathMaker, pieceCompletionForDir(baseDir)) } -func newFileWithCustomPathMakerAndCompletion(baseDir string, pathMaker func(baseDir string, info *metainfo.Info, infoHash metainfo.Hash) string, completion PieceCompletion) ClientImpl { +func newFileWithCustomPathMakerAndCompletion(baseDir string, pathMaker func(baseDir string, info *metainfo.Info, infoHash metainfo.Hash) string, completion PieceCompletion) *fileClientImpl { if pathMaker == nil { pathMaker = defaultPathMaker } diff --git a/storage/interface.go b/storage/interface.go index 307ba8934c9faa6b691865f64079d7b0e96a7c90..49314677bebc8cfc3f193279e703350a10afb7cf 100644 --- a/storage/interface.go +++ b/storage/interface.go @@ -6,10 +6,14 @@ "github.com/anacrolix/torrent/metainfo" ) +type ClientImplCloser interface { + ClientImpl + Close() error +} + // Represents data storage for an unspecified torrent. type ClientImpl interface { OpenTorrent(info *metainfo.Info, infoHash metainfo.Hash) (TorrentImpl, error) - Close() error } // Data storage bound to a torrent. diff --git a/storage/issue96_test.go b/storage/issue96_test.go index 1bdbbf15cb73e7bde6fb701c0e3347b527e14def..50356c7aada5eb71043b249cc30e3df2d3f7251d 100644 --- a/storage/issue96_test.go +++ b/storage/issue96_test.go @@ -10,7 +10,7 @@ "github.com/anacrolix/torrent/metainfo" ) -func testMarkedCompleteMissingOnRead(t *testing.T, csf func(string) ClientImpl) { +func testMarkedCompleteMissingOnRead(t *testing.T, csf func(string) ClientImplCloser) { td, err := ioutil.TempDir("", "") require.NoError(t, err) defer os.RemoveAll(td) diff --git a/storage/mmap.go b/storage/mmap.go index f5437eed517265a2f162123f69fd7bcbe895edad..aebf1094ce7e78c39300502c35c2e9f09b3bbaae 100644 --- a/storage/mmap.go +++ b/storage/mmap.go @@ -19,11 +19,11 @@ baseDir string pc PieceCompletion } -func NewMMap(baseDir string) ClientImpl { +func NewMMap(baseDir string) ClientImplCloser { return NewMMapWithCompletion(baseDir, pieceCompletionForDir(baseDir)) } -func NewMMapWithCompletion(baseDir string, completion PieceCompletion) ClientImpl { +func NewMMapWithCompletion(baseDir string, completion PieceCompletion) *mmapClientImpl { return &mmapClientImpl{ baseDir: baseDir, pc: completion, diff --git a/storage/piece_resource.go b/storage/piece_resource.go index e57e8a8940c78b819accb9ad2bafce230d23fd84..1ef0e602605339b7bd9dce63b6f25cb2acaac411 100644 --- a/storage/piece_resource.go +++ b/storage/piece_resource.go @@ -16,7 +16,7 @@ type piecePerResource struct { p resource.Provider } -func NewResourcePieces(p resource.Provider) ClientImpl { +func NewResourcePieces(p resource.Provider) ClientImplCloser { return &piecePerResource{ p: p, } diff --git a/t.go b/t.go index 82c18bfc841faf99a75fde8ed9b70e27c1424d3c..fcaec919cd0b462475eacaf51e6cecef00366614 100644 --- a/t.go +++ b/t.go @@ -28,8 +28,8 @@ defer t.cl.unlock() return t.info } -// Returns a Reader bound to the torrent's data. All read calls block until -// the data requested is actually available. +// Returns a Reader bound to the torrent's data. All read calls block until the data requested is +// actually available. Note that you probably want to ensure the Torrent Info is available first. func (t *Torrent) NewReader() Reader { r := reader{ mu: t.cl.locker(), diff --git a/test/transfer_test.go b/test/transfer_test.go index dd04dc62893142e6f3e196435ae9ae1efcf84078..7d7e51bbcd482ed5c7c1ca40624a7f64c453bcca 100644 --- a/test/transfer_test.go +++ b/test/transfer_test.go @@ -24,8 +24,8 @@ Responsive bool Readahead int64 SetReadahead bool ExportClientStatus bool - LeecherStorage func(string) storage.ClientImpl - SeederStorage func(string) storage.ClientImpl + LeecherStorage func(string) storage.ClientImplCloser + SeederStorage func(string) storage.ClientImplCloser SeederUploadRateLimiter *rate.Limiter LeecherDownloadRateLimiter *rate.Limiter ConfigureSeeder ConfigureClient @@ -54,8 +54,9 @@ cfg.UploadRateLimiter = ps.SeederUploadRateLimiter } // cfg.ListenAddr = "localhost:4000" if ps.SeederStorage != nil { - cfg.DefaultStorage = ps.SeederStorage(greetingTempDir) - defer cfg.DefaultStorage.Close() + storage := ps.SeederStorage(greetingTempDir) + defer storage.Close() + cfg.DefaultStorage = storage } else { cfg.DataDir = greetingTempDir } @@ -84,7 +85,9 @@ cfg = torrent.TestingConfig() if ps.LeecherStorage == nil { cfg.DataDir = leecherDataDir } else { - cfg.DefaultStorage = ps.LeecherStorage(leecherDataDir) + storage := ps.LeecherStorage(leecherDataDir) + defer storage.Close() + cfg.DefaultStorage = storage } if ps.LeecherDownloadRateLimiter != nil { cfg.DownloadRateLimiter = ps.LeecherDownloadRateLimiter @@ -152,11 +155,11 @@ type fileCacheClientStorageFactoryParams struct { Capacity int64 SetCapacity bool - Wrapper func(*filecache.Cache) storage.ClientImpl + Wrapper func(*filecache.Cache) storage.ClientImplCloser } func newFileCacheClientStorageFactory(ps fileCacheClientStorageFactoryParams) storageFactory { - return func(dataDir string) storage.ClientImpl { + return func(dataDir string) storage.ClientImplCloser { fc, err := filecache.NewCache(dataDir) if err != nil { panic(err) @@ -168,7 +171,7 @@ return ps.Wrapper(fc) } } -type storageFactory func(string) storage.ClientImpl +type storageFactory func(string) storage.ClientImplCloser func TestClientTransferDefault(t *testing.T) { testClientTransfer(t, testClientTransferParams{ @@ -197,7 +200,7 @@ LeecherDownloadRateLimiter: rate.NewLimiter(512, 512), }) } -func fileCachePieceResourceStorage(fc *filecache.Cache) storage.ClientImpl { +func fileCachePieceResourceStorage(fc *filecache.Cache) storage.ClientImplCloser { return storage.NewResourcePieces(fc.AsResourceProvider()) } @@ -245,7 +248,7 @@ t.Run(fmt.Sprintf("LeecherStorage=%s", ls.name), func(t *testing.T) { // Seeder storage for _, ss := range []struct { name string - f func(string) storage.ClientImpl + f func(string) storage.ClientImplCloser }{ {"File", storage.NewFile}, {"Mmap", storage.NewMMap},