0

I am trying to make a method of mine into something that can be called asynchronously.

Normally from the AddQueue Method, I would just call the ListOfJobsInQueue methods in the WorkingSession class, get its result and be done with it.

Using what info I could find regarding Async programming on here, I have done up the below code, but it seems to be getting stuck on the CurrentPageCode property call.

It does not even get to the MessageBox.Show("Processing complete with " + queueResult.Count + " rows"); line.

Could someone please assist and show me where I'm going wrong?

    //Primary Class

    public void AddQueue()
    {
        MessageBox.Show(GetJobsFromQueueAsync().Result.Count().ToString());
    }

    async Task<List<string>> GetJobsFromQueueAsync()
    {
        Task<List<string>> getJobsTask = WorkingSession.GetlistOfJobsAsync();
        List<string> queueResult = await getJobsTask;
        MessageBox.Show("Processing complete with " + queueResult.Count + " rows");
        return queueResult;
    }

    //***

    //WorkingSession Class

    public Task<List<string>> GetlistOfJobsAsync()
    {
        return Task.Run<List<string>>(() =>
        {
            return ListOfJobsInQueue();
        });
    }

    public List<string> ListOfJobsInQueue()
    {
        if (CurrentPageCode == "CS1")
        {
            List<string> actionList = new List<string>();
            short pageNum = PageCurrent;
            short pageMax = PageMax;

            for (short atPage = pageNum; atPage <= pageMax; atPage++)
            {
                //Scan each job on the current queue page
                for (int lineNum = 5; lineNum < 18; lineNum++)
                {
                    string reference = GetJobText(new Coordinate { row = lineNum });
                    actionList.Add(reference);
                }
                //Once finished with this job page, goto the next
                SendCMDKey(Mnemonic.F8);
            }
            return actionList;
        }
        else
        {
            return null;
        }
    }

    //Other method / property signatures (for reference)

    public string CurrentPageCode;
    public bool SendCMDKey(Mnemonic command)
    public string GetJobText(Coordinate coordinate)

    //***
2
  • I'm not sure you can use the async/await approach with your own "async" call can you? (Unless you implement it as a proper/traditional re-entrant function). The examples here: msdn.microsoft.com/en-us/library/hh191443.aspx suggests that the approach you have taken will only work with framework API methods that are designed for async use? Commented Mar 10, 2014 at 0:10
  • 1
    Thats not the impression I got, besides I did another test sample before this project and it worked fine, it was a less complicated scenario though. Commented Mar 10, 2014 at 3:05

2 Answers 2

3

The deadlock problem is actually this method:

public void AddQueue()
{
    MessageBox.Show(GetJobsFromQueueAsync().Result.Count().ToString());
}

Calling Task.Wait or Task<T>.Result should be avoided in async code. I explain the deadlock in full on my blog, but the summary version is that await will capture a context (in this case, the UI context) and attempt to resume its async method on that context (in this case, on the UI thread). With some contexts (e.g., the UI context), if you block a thread in that context (e.g., calling Task<T>.Result on the UI thread), then the async method cannot resume on that context, causing a deadlock.

To fix it, use async all the way:

public async Task AddQueueAsync()
{
    var jobs = await GetJobsFromQueueAsync();
    MessageBox.Show(jobs.Count().ToString());
}

This code is also not ideal, though in a much more subtle way:

public Task<List<string>> GetlistOfJobsAsync()
{
    return Task.Run<List<string>>(() =>
    {
        return ListOfJobsInQueue();
    });
}

By wrapping an entire method's logic in Task.Run, what you're really doing is writing a "fake asynchronous" method. It's got an asynchronous signature but the logic is just synchronous work on a background thread.

It's best to push any Task.Run use as far towards the UI layer as possible; keep it out of any reusable library methods. Make your APIs tell the truth: have synchronous signatures for synchronous work. I have a blog series that goes into detail.

Sign up to request clarification or add additional context in comments.

4 Comments

Given that AddQueue has return type void, and contains UI work (MessageBox.Show) what is the advantage of converting that into async Task AddQueueAsync? I thought such a method was a logical place to use Task.Run to shift into async programming. How about this: public void AddQueue() { Task.Run(() => ListOfJobsInQueue()).ContinueWith((t1) => MessageBox.Show(t1.Result.Count.ToString())); }? Hmm, maybe that doesn't continue on UI thread.
... to explain why I'm interested in not changing the signature of AddQueue: legacy code with thousands of methods over dozens of projects - determining how to add async where its really needed, without having to touch so many methods. There are a lot of void methods; I think a fair number of those can be "fire-and-forget" to avoid "spreading" async through the whole code. Though now I'm seeing this is easy to screw up: its not obvious which methods have side-effects that their callers assume happen before return. I think I've answered my own question. It is safer to let it spread.
@ToolmakerSteve: Yes, it is (usually) safer to let it spread. If you want to do a partial refactoring, check out my sync over async article. "Fire and forget" is dangerous because it can swallow exceptions, in addition to the side effect issues.
If you have a large refactoring, you can draw a line at one point and block like Task.Run(() => ...).GetAwaiter().GetResult() which is safe if ... can run on a background thread. You lose the benefits of asynchrony (since all the "asynchronous" code is called in a blocking fashion), but you get to take a breath in the middle of a large refactor and maybe work on something else for a bit.
0

The simplest that I can

   public async Task<int> GetWorkFlowStageAsync(string tracker,CancellationToken? token = null)
        {
            return await Task.FromResult(0);
        }

1 Comment

When adding a new answer to an old question, please explain what is the benefit of your answer, versus the already existing answer. Also, just posting a piece of code, with no explanation, is rarely useful. What are you demonstrating? What code in the question, or in the existing answer, are you replacing with this snippet of code? When might someone want to use your approach? I don't know if my questions apply to your answer; the important point is to please write down more about your code.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.