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

Fix random Hash Mismatch errors #266

Merged

Conversation

sarpt
Copy link
Contributor

@sarpt sarpt commented Nov 20, 2024

After delivery of #251 I've noticed that when downloading a lot of small packages at once there is quite a high chance of getting a "Hash mismatch" error even though clicking "Download" again and hashing the file second time results in a successful hash calculation suggesting a false negative.
I've noticed that after all write_all calls there doesn't seem to be a flush which awaits until all chunks are actually on disk. Adding this flush seems to have fixed the issue and hashing no longer has a random chance to start and read file size from metadata before everything's on disk.

@RainbowCookie32
Copy link
Owner

Looking at tokio's docs, apparently the right method in this situation would be sync_all. According to the docs, flush just ensures that the file is closed as soon as the handle is dropped.
Confusingly enough, the flush method on std seems to actually flush pending buffers, but it's a no-op on Windows and Unix.

…sh mismatches when hashing after download starts before all chunks are saved to disk
@sarpt sarpt force-pushed the add-flushing-after-write-all branch from 2eef10a to 5182a62 Compare November 21, 2024 22:04
@sarpt
Copy link
Contributor Author

sarpt commented Nov 21, 2024

Looking at tokio's docs, apparently the right method in this situation would be sync_all. According to the docs, flush just ensures that the file is closed as soon as the handle is dropped. Confusingly enough, the flush method on std seems to actually flush pending buffers, but it's a no-op on Windows and Unix.

I changed to sync_all. Seems I mistook std's with Tokio's behavior. Whatever flush did seems it only lessened the probability of the issue instead of fixing it completely then.

@RainbowCookie32 RainbowCookie32 merged commit 2b6ef1f into RainbowCookie32:master Nov 22, 2024
4 checks passed
@sarpt sarpt deleted the add-flushing-after-write-all branch December 2, 2024 17:16
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