1

I have this method

 public override SockResponse Process(Socket socket, Client client) {
        CallResponse rsp = new CallResponse();
        try
        {
            using (Transact X = client.Session.NewTransaction())
            {
                foreach (CallData call in Payload)
                {
                    DataCall dataCall = new DataCall();
                    SqlDataReader rdr = dataCall.Execute(X, call);
                    rsp.Result.Add(dataCall.BuildResult(rdr));
                    rdr.Close();
                }
                rsp.ErrMsg = "";
                X.Commit();
            }
        }
        catch (Exception err)
        {
            rsp.ErrMsg = err.Message;
            return rsp;
        }
        return rsp;
    }

and now I want to make it work asynchronously, and for that I've tried using BackgroundWorker, the new code looks like this

public override SockResponse Process(Socket socket, Client client)
    {
        BackgroundWorker worker = new BackgroundWorker();
        worker.DoWork += new DoWorkEventHandler(
            delegate(object sender, DoWorkEventArgs e)
            {
                CallResponse rsp = new CallResponse();
                try
                {
                    using (Transact X = client.Session.NewTransaction())
                    {
                        foreach (CallData call in Payload)
                        {
                            DataCall dataCall = new DataCall();
                            SqlDataReader rdr = dataCall.Execute(X, call);
                            rsp.Result.Add(dataCall.BuildResult(rdr));
                            rdr.Close();
                        }
                        rsp.ErrMsg = "";
                        X.Commit();
                    }
                }
                catch (Exception err)
                {
                    rsp.ErrMsg = err.Message;
                    e.Result = rsp;
                }
                e.Result = rsp;
            });
        worker.RunWorkerCompleted += new RunWorkerCompletedEventHandler(
            delegate(object sender, RunWorkerCompletedEventArgs e)
            {
                // First, handle the case where an exception was thrown.
                if (e.Error != null)
                {
                    Log.Logger.Error(e.Error.Message);
                }
                else if (e.Cancelled)
                {
                    // Next, handle the case where the user canceled 
                    // the operation.
                    // Note that due to a race condition in 
                    // the DoWork event handler, the Cancelled
                    // flag may not have been set, even though
                    // CancelAsync was called.
                    Log.Logger.Info("CALL process was cancelled");
                }
                else
                {
                    //  Then, handle the result
                    return e.Result; // this is where the problem is
                }
            });
        worker.RunWorkerAsync();
    }

The problem is that I have no idea on how to return the result when the worker is done

Michael Tot Korsgaard
  • 3,892
  • 11
  • 53
  • 89
  • 4
    *Don't* use BGW, it's obsolete and your code shows why - composition is *really* hard. A simple `Task.Run(()=>Process(mySocket,myClient))` will run the method asynchronously. You can use `await` to await for the task and get the result ie `var result=await Task.Run(...);` Even better though is to modify `dataCall.Execute` to use asynchronous methods, eg `ExecuteReaderAsync` – Panagiotis Kanavos Oct 26 '16 at 08:39
  • Yes, Bgw isn't the best approach. It is sometimes the easiest though, but you've got to follow the steps. Your Try/catch completely circumvents the `if (e.Error != null)` code. And you can't return anything from the completed handler, you just store it somewhere (you're running on the main thread again). – H H Oct 26 '16 at 09:14

2 Answers2

2

It appears you want to run a async code inside a synchronous method. I.e. you have a method which returns a SocketResponse, and want to make it asynchronous.

Well, here's the problem. Basically - you can't. Or at least it's not as simple as just making use of BackgroundWorker.

One way to do this is to also employ a TaskCompletionSource, and then wait for it to finish. The code would look like this:

public override SockResponse Process(Socket socket, Client client)
{
    BackgroundWorker worker = new BackgroundWorker();
    worker.DoWork += new DoWorkEventHandler(
        delegate (object sender, DoWorkEventArgs e)
        {
            CallResponse rsp = new CallResponse();
            try
            {
                using (Transact X = client.Session.NewTransaction())
                {
                    foreach (CallData call in Payload)
                    {
                        DataCall dataCall = new DataCall();
                        SqlDataReader rdr = dataCall.Execute(X, call);
                        rsp.Result.Add(dataCall.BuildResult(rdr));
                        rdr.Close();
                    }
                    rsp.ErrMsg = "";
                    X.Commit();
                }
            }
            catch (Exception err)
            {
                rsp.ErrMsg = err.Message;
                e.Result = rsp;
            }
            e.Result = rsp;
        });
    TaskCompletionSource<SockResponse> tcs = new TaskCompletionSource<SockResponse>();
    worker.RunWorkerCompleted += new RunWorkerCompletedEventHandler(
        delegate (object sender, RunWorkerCompletedEventArgs e)
        {
            // First, handle the case where an exception was thrown.
            if (e.Error != null)
            {
                Log.Logger.Error(e.Error.Message);
            }
            else if (e.Cancelled)
            {
                // Next, handle the case where the user canceled 
                // the operation.
                // Note that due to a race condition in 
                // the DoWork event handler, the Cancelled
                // flag may not have been set, even though
                // CancelAsync was called.
                Log.Logger.Info("CALL process was cancelled");
            }
            else
            {
                //  Then, handle the result
                tcs.SetResult(e.Result);
            }
        });
    worker.RunWorkerAsync();
    return tcs.Task.Result;
}

This is an ugly approach - your Process method might be running code in a separate thread, but will otherwise take a long time to execute. The use of BackgroundWorker in this case is also overkill (a simple Task would work just the same in this case). A simplified version of the code using a Task would be:

public override SockResponse Process(Socket socket, Client client)
{
    return Task.Factory.StartNew(() =>
    {
        CallResponse rsp = new CallResponse();
        try
        {
            using (Transact X = client.Session.NewTransaction())
            {
                foreach (CallData call in Payload)
                {
                    DataCall dataCall = new DataCall();
                    SqlDataReader rdr = dataCall.Execute(X, call);
                    rsp.Result.Add(dataCall.BuildResult(rdr));
                    rdr.Close();
                }
                rsp.ErrMsg = "";
                X.Commit();
            }
        }
        catch (Exception err)
        {
            rsp.ErrMsg = err.Message;
            return rsp;
        }
        return rsp;
    }).Result;
}

The above is running code in a new thread, but the Process method will still take a lot of time.

A BETTER approach is to make use of the async/await keywords and rewrite the method to return a Task. This MIGHT require that you rewrite your application in more places than one.

Here's how that same method might look like as an async method:

public async Task<SockResponse> ProcessAsync(Socket socket, Client client)
{
    var task = Task.Factory.StartNew(() =>
    {
        CallResponse rsp = new CallResponse();
        try
        {
            using (Transact X = client.Session.NewTransaction())
            {
                foreach (CallData call in Payload)
                {
                    DataCall dataCall = new DataCall();
                    SqlDataReader rdr = dataCall.Execute(X, call);
                    rsp.Result.Add(dataCall.BuildResult(rdr));
                    rdr.Close();
                }
                rsp.ErrMsg = "";
                X.Commit();
            }
        }
        catch (Exception err)
        {
            rsp.ErrMsg = err.Message;
            return rsp;
        }
        return rsp;
    });
    return await task;
}

HOWEVER, I already see potential issues with this: your Process method is an override, meaning you can't just make it return a Task, and you can't just slap on the async keyword on it either.

So, depending on how you're running the Process method, perhaps you can simply run that in a separate thread? Something along the lines of:

var socketResponse = await Task.Factory.StartNew(() => Process(socket, client));

or (if not using async/await)

var socketResponse = Task.Factory.StartNew(() => Process(socket, client)).Result;

Also, unless you're overriding your own base class (meaning you can rewrite code to make it async), then it seems like you're working with sockets - code related to Socket use is, by default, based on events (Begin/End methods), and should be naturally "asynchronous".

MBender
  • 5,395
  • 1
  • 42
  • 69
-1

To return a value from BackgroundWorker, consider following MSDN article: MSDN

Look down to the sample code and read following:

worker.ReportProgress(i * 10);

Or, to report a Object, consider the following StackOverflow article: How to make BackgroundWorker return an object

private void bgw_DoWork(object sender, DoWorkEventArgs e)
{
    int result = 2+2;
    e.Result = result;
}

private void bgw_RunWorkerCompleted(object sender, RunWorkerCompletedEventArgs e)
{
    int result = (int)e.Result;
    MessageBox.Show("Result received: " + result.ToString());
}
Community
  • 1
  • 1
Radinator
  • 1,048
  • 18
  • 58
  • Making use of `RunWorkerCompleted` to return a result is a valid way to... well... return results. The OP is already using it. Your suggestion to use `ReportProgress` doesn't solve the issue in the question at all. – MBender Oct 26 '16 at 09:11