C#•2y ago

❔ ✅ Baffled by Async-Await and Task

I'm trying to create a non-UI blocking Unzipper helper class that reports progress. I've tried to take on async/await + tasks so many times, but always come a cropper. I can manage Async/await with .net async methods (e.g. webClient.DownloadFileAsync, but the problem comes with sychronous key methods. Anyways, here's the helper class: https://paste.mod.gg/xklfwuocvtgm/0 and it's called, thus:

 string targetDir = "C:\\temp\\project tests\\Updater\\NetUpdate\\App";
Unzipper unzipper = new Unzipper(new FileInfo(tempPayloadFile), new DirectoryInfo(targetDir));
unzipper.NewEntryExtractionStarted += Unzip_NewEntryExtractionStarted;
unzipper.EntryExtractionProgressed += Unzip_EntryExtractionProgressed;
unzipper.Unzip();

 string targetDir = "C:\\temp\\project tests\\Updater\\NetUpdate\\App";
Unzipper unzipper = new Unzipper(new FileInfo(tempPayloadFile), new DirectoryInfo(targetDir));
unzipper.NewEntryExtractionStarted += Unzip_NewEntryExtractionStarted;
unzipper.EntryExtractionProgressed += Unzip_EntryExtractionProgressed;
unzipper.Unzip();

I've done no end of reading - just conceptually this is illuding me. What am I missing - based on this code?

BlazeBin - xklfwuocvtgm

A tool for sharing your source code with the world!

58 Replies

Angius•2y ago

Well, one thing you're missing, is that async methods need to be awaited .Wait() is blocking .GetAwaiter().GetResult() is blocking .Result is blocking Only await isn't

stigzlerOP•2y ago

🤣 ah - not much wrong with it then

Angius•2y ago

As a side note, WebClient is obsolete. Has been for years now. Use HttpClient instead

mtreit•2y ago

 Task unzipTask = new Task( () =>  PerformUnzip(archiveFile.FullName, targetDirectory.FullName, unzipProgress));

 Task unzipTask = new Task( () =>  PerformUnzip(archiveFile.FullName, targetDirectory.FullName, unzipProgress));

Why are you creating a new task here? PereformUnzip already returns a task

stigzlerOP•2y ago

firstly - just to give you scope - I'm an amayeur coder - so it's just a hobby

mtreit•2y ago

In general, don't use new to create tasks.

stigzlerOP•2y ago

I was following the MS example: https://learn.microsoft.com/en-us/dotnet/standard/parallel-programming/task-based-asynchronous-programming

Task-based asynchronous programming - .NET

In this article, learn about task-based asynchronous programming through the Task Parallel Library (TPL) in .NET.

mtreit•2y ago

As that document also says, using Task.Run is the preferred way to create and start tasks. In some rare case you might want a "cold" task that doesn't run until you explicitly call Start on it, but I think that is incredibly uncommon. Anyway, I don't see any async code anywhere in what you posted.

stigzlerOP•2y ago

My previous incarnation was thus::

        public async void Unzip()
        {
            var unzipProgress = new Progress<ExtractProgressEventArgs>();
            unzipProgress.ProgressChanged += Progress_ProgressChanged;

            await Task.Run(() => PerformUnzip(archiveFile.FullName, targetDirectory.FullName, unzipProgress).ConfigureAwait(false));

            unzipProgress.ProgressChanged -= Progress_ProgressChanged;
        }

        private async Task PerformUnzip(string payloadFile, string targetDir, IProgress<ExtractProgressEventArgs> progress)
        {
            ZipFile zFile = new ZipFile(payloadFile);

            zFile.ExtractProgress += (s, e) => { progress.Report(e); };

            zFile.ExtractAll(targetDir, ExtractExistingFileAction.OverwriteSilently);

        }

        public async void Unzip()
        {
            var unzipProgress = new Progress<ExtractProgressEventArgs>();
            unzipProgress.ProgressChanged += Progress_ProgressChanged;

            await Task.Run(() => PerformUnzip(archiveFile.FullName, targetDirectory.FullName, unzipProgress).ConfigureAwait(false));

            unzipProgress.ProgressChanged -= Progress_ProgressChanged;
        }

        private async Task PerformUnzip(string payloadFile, string targetDir, IProgress<ExtractProgressEventArgs> progress)
        {
            ZipFile zFile = new ZipFile(payloadFile);

            zFile.ExtractProgress += (s, e) => { progress.Report(e); };

            zFile.ExtractAll(targetDir, ExtractExistingFileAction.OverwriteSilently);

        }

I was just trying a different approach with the posted code

Angius•2y ago

Well, it all boils down to all those wrappers you're making not wrapping any actually asynchronous code You can be adding layers and layers of tasks, sure, but all you'll get out of it is overhead

mtreit•2y ago

Where does that ZipFile class come from? The one from System.IO.Compression does not have an ExtractAll method?

stigzlerOP•2y ago

I thought so - so how do I make zFile.ExtractAll asynchronous? no it's DotNetZip but I'm interested more broadly in how you make synchronous methods asynchornous or essentially run on a different thread

Angius•2y ago

Async != multithreading

mtreit•2y ago

You don't

stigzlerOP•2y ago

thus my exploring tasks

Angius•2y ago

Async/await is about efficient use of a single thread

mtreit•2y ago

ZipFile.ExtractAll is not async. There is not really a way to make it async if the implementation is entirely synchronous.

Angius•2y ago

Not about multithreading

stigzlerOP•2y ago

yes - I think I get that - I would need the unzip to run on a different thread to the UI thread and I would have to run it asnyc to await it finishing as would need to finish extracting before moving onto another stage in the overall process of the app process

mtreit•2y ago

You don't need to use async at all. Async is not buying you anything if there is no actual async I/O happening. You just need to be notified when the task is done.

stigzlerOP•2y ago

forgive me mtreit - I did explain above - I'm an amateur coder and I've never got my head around that bit, either hear me out let's say my main method is: Download Zip File > Unzip Files > Copy Files to Location and this all sits in a main method:

DownloadZip();
UnzipFiles();
CopyFiles();

DownloadZip();
UnzipFiles();
CopyFiles();

obviously, CopyFiles would have to wait until Unzip files is completed. Thus you'd need some way to wait for UnzipFiles to complete. You copuldn't do this with a loop + "Application.DoEvents" for example - that's why I thought you were meant to use async/await? Also - I don't see how you'd use Events in this scenario? Unless you trigger methods from events?

Angius•2y ago

var zip = await DownloadZip();
var files = await UnzipFiles(zip);
await CopyFiles(files, "C://MyFiles");

var zip = await DownloadZip();
var files = await UnzipFiles(zip);
await CopyFiles(files, "C://MyFiles");

await is what waits But only if the method is actually asynchronous If it isn't, it's just

var zip = await DownloadZip();
var files = UnzipFiles(zip);
await CopyFiles(files, "C://MyFiles");

var zip = await DownloadZip();
var files = UnzipFiles(zip);
await CopyFiles(files, "C://MyFiles");

mtreit•2y ago

If DownloadZip() and UnzipFiles() and CopyFiles() do not use async inside of them there is no reason to try to use async / await at all. If you want your main thread to be able to do other stuff while the above code is in progress, run it in a task via Task.Run And have the task raise an event or set some flag or something when it's done (or just check the task status)

stigzlerOP•2y ago

But that's what I do in the code above and it still blocks the UI

mtreit•2y ago

Yeah, don't call Wait That blocks

stigzlerOP•2y ago

OK - one sec I'm sure I tried taking out all the await async, but will try again dammit - nope - still blocking main UI thread I'm wondering whether it's something to do with events or Task.Report her's the Zip helper as stands

stigzlerOP•2y ago

https://paste.mod.gg/dmqmfdjgajhr/0

BlazeBin - dmqmfdjgajhr

A tool for sharing your source code with the world!

stigzlerOP•2y ago

and an extract of caller and related handlers

stigzlerOP•2y ago

https://paste.mod.gg/dmqmfdjgajhr/2

BlazeBin - dmqmfdjgajhr

A tool for sharing your source code with the world!

mtreit•2y ago

How are you executing this code in your UI thread?

stigzlerOP•2y ago

sorry - what do you mean?

mtreit•2y ago

You said this is blocking your UI thread.

stigzlerOP•2y ago

ah- OK. I'm developing a class library (a software update library). I have a test winforms project setup to receive percent complete and a couple of messages via the UpdateProgressChanged Event

mtreit•2y ago

What does the winforms code look like?

stigzlerOP•2y ago

The full main code is here (very long/un optimised): https://paste.mod.gg/nfceguxamfcm/3

mtreit•2y ago

That's the unzipper you already shared

stigzlerOP•2y ago

click the page tab at the top FullMainCLass

mtreit•2y ago

Oh, I didn't even know paste.mod.gg supported more than one file at a time 🙂

stigzlerOP•2y ago

🙂 form code behind posted - pretty straight forward

mtreit•2y ago

Doesn't look like it should block anything At first glance You are doing all sorts of bad stuff like starting tasks and throwing them away and using things like async void that you should pretty much never do if it's not an event handler... But I don't see where the code would block

stigzlerOP•2y ago

Deffo is 🤷

mtreit•2y ago

What's the call stack when it "blocks" ?

stigzlerOP•2y ago

Think I may have cracked it. The ProgressBar and Label message were getting updated. I swapped new instances of the EventArgs for some private member EventArgs variables defined at a class level (thus on the main thread). This worked:

        private void Unzip_EntryExtractionProgressed(EntryExtractionProgressedEventArgs e)
        {
            //Debug.WriteLine("Unzip_EntryExtractionProgressed fired");
            opProgressArgs.PercentageComplete = (int)((e.BytesTransferred / e.TotalBytesTransferred) * 100);

            //UpdateProgressChanged.Invoke(new OperationProgressChangedEventArgs
            //{
            //    //PercentageComplete = (int)((e.BytesTransferred / e.TotalBytesTransferred) * 100)
            //});
            UpdateProgressChanged.Invoke(opProgressArgs);
        }

        private void Unzip_NewEntryExtractionStarted(NewEntryExtractionStartedEventArgs e)
        {
            Debug.WriteLine("Unzip_NewEntryExtractionStarted fired");
            opProgressArgs.UpdateMessage = "Entry " + e.EntriesExtracted + " of " + e.TotalEntries + ": " + Path.GetFileName(e.CurrentEntry.FileName);
            opProgressArgs.PercentageComplete = 0;
            //UpdateProgressChanged.Invoke(new OperationProgressChangedEventArgs
            //{
            //    UpdateMessage = "Entry " + e.EntriesExtracted + " of " + e.TotalEntries + ": " + Path.GetFileName(e.CurrentEntry.FileName)
            //});
            UpdateProgressChanged.Invoke(opProgressArgs);
        }

        private void Unzip_EntryExtractionProgressed(EntryExtractionProgressedEventArgs e)
        {
            //Debug.WriteLine("Unzip_EntryExtractionProgressed fired");
            opProgressArgs.PercentageComplete = (int)((e.BytesTransferred / e.TotalBytesTransferred) * 100);

            //UpdateProgressChanged.Invoke(new OperationProgressChangedEventArgs
            //{
            //    //PercentageComplete = (int)((e.BytesTransferred / e.TotalBytesTransferred) * 100)
            //});
            UpdateProgressChanged.Invoke(opProgressArgs);
        }

        private void Unzip_NewEntryExtractionStarted(NewEntryExtractionStartedEventArgs e)
        {
            Debug.WriteLine("Unzip_NewEntryExtractionStarted fired");
            opProgressArgs.UpdateMessage = "Entry " + e.EntriesExtracted + " of " + e.TotalEntries + ": " + Path.GetFileName(e.CurrentEntry.FileName);
            opProgressArgs.PercentageComplete = 0;
            //UpdateProgressChanged.Invoke(new OperationProgressChangedEventArgs
            //{
            //    UpdateMessage = "Entry " + e.EntriesExtracted + " of " + e.TotalEntries + ": " + Path.GetFileName(e.CurrentEntry.FileName)
            //});
            UpdateProgressChanged.Invoke(opProgressArgs);
        }

Still feels a little bit sticky, but think it's working So that's part one solved But second part of the question is if I wanted to extend this method further, how would I wait until the unzip have finished? If I do it via an event - it could set a flag - but how would I loop whilst monitoring for the flag change? What's best practice? I.e:

string targetDir = "C:\\temp\\project tests\\Updater\\NetUpdate\\App";

Unzipper unzipper = new Unzipper(new FileInfo(tempPayloadFile), new DirectoryInfo(targetDir));
unzipper.NewEntryExtractionStarted += Unzip_NewEntryExtractionStarted;
unzipper.EntryExtractionProgressed += Unzip_EntryExtractionProgressed;
unzipper.Unzip();

// Loop here until Unzip finished and flag set - how?

MoveFiles();

string targetDir = "C:\\temp\\project tests\\Updater\\NetUpdate\\App";

Unzipper unzipper = new Unzipper(new FileInfo(tempPayloadFile), new DirectoryInfo(targetDir));
unzipper.NewEntryExtractionStarted += Unzip_NewEntryExtractionStarted;
unzipper.EntryExtractionProgressed += Unzip_EntryExtractionProgressed;
unzipper.Unzip();

// Loop here until Unzip finished and flag set - how?

MoveFiles();

mtreit•2y ago

I don't generally write UI programs, you might want to ask in #gui what the standard way to do that is. My gut feeling it to just fire an event and have the event handler do whatever needs to happen next.

stigzlerOP•2y ago

I guess it's not technically gui related as it's in a class library isn't this now falling into the realms of await?

mtreit•2y ago

Since nowhere in your code do you perform async I/O, no it doesn't fall into the realm of await.

stigzlerOP•2y ago

the task seems to be how to wait for a task to finish before proceeding without blocking a thread

mtreit•2y ago

You can check if the task is finished. If not, go do other stuff. Although I thought in UI programs you generally use event handlers

stigzlerOP•2y ago

So, as I'm imagining this, for each long running task on a different thread, you would listen for it's completed event then call the next stage in the longer process (in this example:

private void UnzipFinishedHandler()
{
  MoveFiles()
}

private void UnzipFinishedHandler()
{
  MoveFiles()
}

Is this acceptable practice? 🤯 Thanks for all your help

mtreit•2y ago

That might work. You can also look at using Task.ContinueWith You can chain a bunch of tasks together using ContinueWith to get all of the work done.

stigzlerOP•2y ago

Just an update in case anyone comes back to this Turns out the solution did involve async/await. Implemented it in Unzipper.cs and you now just use the below to await a synchronous method (in this case DotNetZip's unzip) asynchronously - ie. its not blocking the thread

await unzipper.Unzip();

await unzipper.Unzip();

stigzlerOP•2y ago

Unzipper code:@ https://paste.mod.gg/knxfcuhjodix/0

BlazeBin - knxfcuhjodix

A tool for sharing your source code with the world!

mtreit•2y ago

You might want to read this: https://blog.stephencleary.com/2013/11/taskrun-etiquette-examples-dont-use.html It describes your scenario I think: not wanting to block the UI thread and therefore introducing an async wrapper over what is really a synchronous operation. The solution in the post (which is by Stephen Cleary who is well known as an expert on this topic) is to move the await out of the library code and into the UI code, where it can explicitly start a task and await it. Rather than having all of that goop in your library code.

Task.Run Etiquette Examples: Don't Use Task.Run in the Implementation

Last time we looked at using Task.Run for the wrong thing (code that is not CPU-bound). So let’s move on to the proper use case of Task.Run: CPU-bound code. We start off with some existing code, which synchronously does some heavy calculations.

mtreit•2y ago

He also points to this, which is another good post on the topic: https://devblogs.microsoft.com/pfxteam/should-i-expose-asynchronous-wrappers-for-synchronous-methods/?WT.mc_id=DT-MVP-5000058

.NET Parallel Programming

Should I expose asynchronous wrappers for synchronous methods?

Lately I’ve received several questions along the lines of the following, which I typically summarize as “async over sync”: In my library, I have a method “public T Foo();”. I’m considering exposing an asynchronous method that would simply wrap the synchronous one,

stigzlerOP•2y ago

Hey thanks - I'll take a look I do get the point - it was one of my considerations - maybe it should be the developer's responsibility to run my library on a separate thread rather than my responsibility to do it for them (although I'm only developing it for myself 🤣)

Accord•2y ago

Was this issue resolved? If so, run /close - otherwise I will mark this as stale and this post will be archived until there is new activity.

stigzlerOP•2y ago

In the end - I followed @mtreit 's advice and took all async/multi-thread code (where I could) out of the library and moved that responsibility to the calling thread. For example - this is now achieved from the UI thread on a different one async via await Task.Run(() => netUpdate.UpdateApplication(progress))

Accord•2y ago

Was this issue resolved? If so, run /close - otherwise I will mark this as stale and this post will be archived until there is new activity.

Gaming

Programming

❔ ✅ Baffled by Async-Await and Task