Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure hash checking a file is a purely read-only operation #674

Merged
merged 5 commits into from
Aug 19, 2024

Conversation

alanmcgovern
Copy link
Owner

The original implementation had some conveniences which are not great in certain circumstances. The DiskManager implementation would do it's utmost to ensure files were always the correct size/length, and that zero length files always existed on disk.

This is fine except if someone wishes to check if a particular file matches a particular hash - in this scenario, while they simply wanted to see if there was a match, the act of running the hashcheck would also truncate the file if it were too large.

Now the engine doesn't do this, and TorrentManager.HashCheckAsync neither truncates, nor creates zero length files.

If someone takes the step of actually calling TorrentManager.StartAsync, this is a clear sign they really do want to download the torrent, and at that point zero length files will be created, and existing files will be truncated if they are too large. This operation occurs only during the 'Starting' phase, prior to enter 'Downloading' or 'Seeding'. This makes things a little easier to reason about :)

However, to support this in the optimal way additional methods need to be added to some of the interfaces used for IO, such as a get/set length method.

@ManlyMarco
Copy link

ManlyMarco commented Aug 10, 2024

This is great, it should fix all of my issues. The only thing that would make it better is letting HashCheckAsync use fast resume data.

If you load fast resume data then the "NeedsHashCheck" Boolean is set to false. When you call StartAsync no additional hash check will be performed.

If you don't load fast resume, the library calls HashCheckAsync implicitly when "StartAsync" is invoked to perform a hashcheck.

From an external user perspective - don't call HashCheckAsync unless you want a full file-validating hashcheck to be performed. Either rely on implicit or explicit save/restore of fastresume data or let "StartAsync" do what it needs to do :)

@alanmcgovern alanmcgovern force-pushed the readonly-hashing branch 7 times, most recently from 17315d2 to 256d288 Compare August 17, 2024 09:56
The engine can try to create sparse files, or fully preallocate files,
when downloading a torrent.
CreateAsync
GetLengthAsync
SetLengthASync

These abstract away most of the remaining IO operations.
This works exactly how you'd expect for BitTorrent v2 torrents. For
BitTorrent v1 torrents we still need to create files when a piece we
need to download spans across two, or more, files marked
'DoNotDownload'.

I've low interest in implementing support for placeholder files when
this situation occurs.
If any file is missing the torrent will not be marked complete. Missing
zero length files don't impact partial seeding status as no-one would
try downloading them.
@alanmcgovern alanmcgovern merged commit d60d2ae into master Aug 19, 2024
2 checks passed
@alanmcgovern alanmcgovern deleted the readonly-hashing branch August 19, 2024 07:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants