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

[PieceWriter] Fix a threading bug and clean up stale state #629

Merged
merged 3 commits into from
Feb 6, 2023

Conversation

alanmcgovern
Copy link
Owner

@alanmcgovern alanmcgovern commented Feb 6, 2023

Several small improvements to the PieceWriter code.

  1. Fix a thread safety issue caused by an incorrect 'ConfigureAwait(false)' as part of the BEP47 padding file support.
  2. Simplify threading concerns when disposing streams.
  3. Ensure state is cleaned up fully after closing the final stream associated with a given ITorrentFile.

The threading fix in point 1 may help with #627

This can be safely disposed on a background thread as the
underlying stream cannot be in use anywhere else at this
point.
@alanmcgovern alanmcgovern changed the title [PieceWriter] [PieceWriter] Fix a threading bug and clean up stale state Feb 6, 2023
Whoops - there was a 'ConfigureAwait(false)' here which caused
improper threaded access to the underlying PieceWriter. The
failure mode was that if there was a torrent with hundreds of
tiny files, multiple threads could be opening/closing streams
at the same time and cause issues due to modifying the underlying
'Streams' dictionary while enumerating it.

i.e.
This exception was originally thrown at this call stack:
    System.ThrowHelper.ThrowArgumentOutOfRange_IndexException()
    System.Collections.Generic.List<T>.RemoveAt(int)
    System.Collections.Generic.List<T>.Remove(T)
    MonoTorrent.PieceWriter.DiskWriter.MaybeRemoveOldestStreams() in DiskWriter.cs
    MonoTorrent.PieceWriter.DiskWriter.GetOrCreateStreamAsync(MonoTorrent.ITorrentManagerFile, System.IO.FileAccess) in DiskWriter.cs
    System.Runtime.CompilerServices.ReusableTaskAwaiter<T>.GetResult()
    MonoTorrent.PieceWriter.DiskWriter.ReadAsync(MonoTorrent.ITorrentManagerFile, long, System.Memory<byte>) in DiskWriter.cs
DiskWriter has a dictionary mapping ITorrentFile -> list of open
streams. When the final stream has been closed, remove the key
from the dictionary.
@alanmcgovern alanmcgovern force-pushed the cleanup-some-more-filestream-stuff branch from 289e3d2 to 60a94b2 Compare February 6, 2023 14:01
@alanmcgovern alanmcgovern merged commit 7931689 into master Feb 6, 2023
@alanmcgovern alanmcgovern deleted the cleanup-some-more-filestream-stuff branch February 6, 2023 14:17
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.

1 participant