Geek Answers Handbook

The correct structure for reading async FileStream

I have a custom list control that displays items with thumbnails of images. Each list item is provided with the full path to the file and it asynchronously reads it using FileStream.BeginRead, and it needs to cancel the list control when the file is finished reading.

At any time, the list can also be cleared of items and re-populated with various items. This causes Dispose for each element that should gracefully handle filter deletion (which may still be in the middle of reading async).

I will show the code that I use. I am not sure about the correct use of calls and blocking objects in such a situation when a request to download a new file asynchronously may appear when another file is in the middle of the download asynchronously.

public string FileName { get; set; } public Image Image { get; set; } public Control Parent { get; set; } private FileStream currentFileStream; private byte[] buffer; private object locker = new object(); private bool loading; private bool disposed; public void LoadImage(string fileName) { FileName = fileName; lock (locker) { currentFileStream = new FileStream(FileName, FileMode.Open, FileAccess.Read, FileShare.Read); buffer = new byte[currentFileStream.Length]; currentFileStream.BeginRead(buffer, 0, buffer.Length, FileReadComplete, currentFileStream); loading = true; } } private void FileReadComplete(IAsyncResult ar) { FileStream fileStreamComplete = (FileStream)ar.AsyncState; lock (locker) { fileStreamComplete.EndRead(ar); // If the finished FileStream is the more recent one requested // And this item has not been disposed if (fileStreamComplete == currentFileStream && !disposed) { try { loading = false; Image = new Bitmap(currentFileStream); currentFileStream.Close(); currentFileStream.Dispose(); Parent.Invalidate(); } catch (Exception e) { } finally { currentFileStream = null; } } else { fileStreamComplete.Close(); fileStreamComplete.Dispose(); } } } protected override void Dispose(bool disposing) { lock (locker) { base.Dispose(disposing); if (!disposed) { if (disposing) { if (Image != null) Image.Dispose(); } disposed = true; } } }

EDIT : Removed the disposal of currentFileStream from the Dispose () method.

EDIT # 2 : Removed the disposal of currentFileStream from the LoadImage () function. This probably should not be, since the file can continue to be read and cannot be closed during the operation. It will be configured regardless of when the FileReadComplete callback is called.

+4
source share
1 answer

I'm not sure I followed your code, but this is what I would recommend.

  • Use a different FileStream constructor if you plan to use it as read-only async. However, this may not be related to your problem, but this is the best way to do this.

     currentFileStream = new FileStream(FileName, FileMode.Open, FileAccess.Read, FileShare.Read, 1024 * 8, true);

  • Any reason you don't want to specify the path to the Bitmap constructor (or Image.FromFile) and upload the file? Remember that when loading a large number of files into memory, their sequential download may be faster (in case the files are based on sequential access technology, for example, on a hard disk).

Assuming you want to load it asynchronously anyway, I just encapsulate this functionality in a class for "accuracy"

It seems you are loading your image from the same stream that you already read in your buffer. I am sure this is a problem. Below is my adaptation of your code. Major changes

  • I do not use the variable currentFileStream
  • dispose variable becomes volatile as it can be accessed from multiple threads
  • Calls in FileStream.Dispose are redundant after FileStream.Close, therefore deleted.

I have not tried the code, so you should tell me if it works

 class ImageLoader : IDisposable { public string FileName { get; set; } public Image Image { get; set; } public Control Parent { get; set; } private FileStream currentFileStream; private byte[] buffer; private object locker = new object(); Control parent; private volatile bool dispose = false; public ImageLoader(Control parent, string fileName) { Parent = parent; FileName = fileName; Image = null; currentFileStream = new FileStream(FileName, FileMode.Open, FileAccess.Read, FileShare.Read, 1024 * 8, true); buffer = new byte[currentFileStream.Length]; currentFileStream.BeginRead(buffer, 0, buffer.Length, new AsyncCallback(FileReadComplete), null); } private void FileReadComplete(IAsyncResult ar) { lock (locker) { try { currentFileStream.EndRead(ar); } catch (ObjectDisposedException) { } if (!dispose) { using (MemoryStream ms = new MemoryStream(buffer)) Image = new Bitmap(ms); Parent.Invalidate(); } try { currentFileStream.Close(); } catch(IOException) { } } } public void Dispose() { lock (locker) { if (dispose) return; dispose = true; try { currentFileStream.Close(); } catch(IOException) { } if (Image != null) Image.Dispose(); } } }

EDIT1: Based on your comments, I am adding an answer here, as the system does not allow me to add text there

  • Encapsulating such functions in a class (set of functions) or function is good consistency, and in many cases such good practice has performance penalties. You should use it depending on your situation.
  • I don't think this is how streams work, but in the case of small files, I see how it will be. To explain this, if you specify a buffer size for files, for example, 8K, a file larger than 8K cannot be cached in memory. Thus, the stream cannot read the entire file without real I / O (forget about what Windows does behind the screen for this). In addition, the Bitmap constructor can potentially perform synchronous I / O, and problems can occur when it is opened in asynchronous mode. MSDN clearly states that a thread should only be used in one mode (synchronous or asynchronous) throughout the life cycle of stream objects. I strongly believe that you should close the stream and read from your own buffer that you have already created. I would say that this works for you, this does not mean that it will work in another scenario; you can just be "happy."
  • I agree with the volatile keyword, I just practice it this way, because sometimes when changing the code (for example, I get rid of the lock all together) this saves me from my mistakes.
+1
source

More articles:


All Articles