1

I am trying to write a function which uses Task and TaskCompletion.

My problem is that after login, the result is not returned. I used similar code before and it was working. I do not know what causes for this situation.

public async Task<byte[]> Sign(byte[] documentContent)
{
    var service = new SignServiceWrapper("https://example.com?wsdl");
    var loginResult = await Task.Run(() => service.Login(loginRequest));

    //....
}

and my SignServiceWrapper class

public class SignServiceWrapper
{
    private static string _webServiceUrl;
    private  BrokerClientClient client;

    public SignServiceWrapper(string webServiceUrl)
    {
        _webServiceUrl = webServiceUrl;
    }

    public Task<loginResponse> Login(loginRequest request)
    {
        var tcs = new TaskCompletionSource<loginResponse>();

        ClientGenerator.WebServiceUrl = _webServiceUrl;

        ClientGenerator.InitializeService(); 
        client = ClientGenerator.ServiceClient;   

        client.loginCompleted += (sender, loginResult) =>
        {
            if (loginResult.Error != null)
                tcs.SetException(loginResult.Error);
            else
                tcs.TrySetResult(loginResult.Result);
        };    

        client.loginAsync(request);                

        return tcs.Task;
    }

    // ...
}

If I call my login function like that it works

var loginResult = Task.Run(() => service.Login(loginRequest));
loginResult.Wait();

I know that there is kind of a deadlock but I don't know how to solve this here and which object.

Manfred Radlwimmer
  • 13,257
  • 13
  • 53
  • 62
ertan2002
  • 1,458
  • 1
  • 32
  • 68
  • How long have you waited for the task to complete? Have you tried stepping through the task to make sure it is processing? – Takarii Jan 18 '17 at 09:02
  • I put a breakpoint in loginCompleted event and the result comes there but problem is it does not return – ertan2002 Jan 18 '17 at 09:03
  • `await` isn't broken. Your code though is rather unusual - Task.Run *and* TCS *and* web service calls, that already has a `loginAsync` method? Why don't you remove the *entire* `Login` method and just call `var response=await client.loginAsync(request);` ? – Panagiotis Kanavos Jan 18 '17 at 09:10
  • Note that using `Task.Run` and then making a blocking call inside it results in fake asynchrony. WCF's asynchronous methods *don't use threads at all*. Windows IO is actually asynchronous at the kernel level with blocking simulated to make programming easier. Using `Task.Run` on top of a blocking method wastes a background thread. Using the WCF proxy's Async or Begin/End methods doesn't – Panagiotis Kanavos Jan 18 '17 at 09:12
  • I generate a proxy class from webservice and it contains only async calls like that. your code that you give, loginAsync is a void method, it does not return anything so i have to use loginCompleted event to get result.. so for that i have to use TaskCompletion – ertan2002 Jan 18 '17 at 09:15
  • Post your code then. Why *don't* you use WCF's proxies? WCF generates asynchronous methods already - Task-based since 2013, Begin/Async earlier. ASMX services also generate Begin/End methods, probably Task-based too. It looks like your code is trying to reverse whatever the proxy is doing and get back to the original asynchronous methods – Panagiotis Kanavos Jan 18 '17 at 09:16
  • Well I used silverlight util tool to create proxy from the webservice (i do not want to add the service as a service reference) maybe i should do like that – ertan2002 Jan 18 '17 at 09:30
  • What is the implementation of the `LoginClient.login` service, I want to see how it's `login` method is implemented. – David Pine Jan 18 '17 at 12:03
  • Please tell us **exactly** what you mean by "is not returned". Are you saying that it does not return, at all, period? Or that it does return but returns the wrong value? In other words, is the problem a deadlock/lost-in-space task, or wrong return value? – Lasse V. Karlsen Jan 18 '17 at 12:47
  • @DavidPine, I cant show, because i dont have the webservice's code. – ertan2002 Jan 18 '17 at 13:38
  • @LasseV.Karlsen, It means var loginResult = Task.Run(() => service.Login(loginRequest)).Result; after this method, the program does not cycle, it waits there even the completed event is triggered.. – ertan2002 Jan 18 '17 at 13:39
  • So for now I use the code like that and it works var loginResult = Task.Run(() => service.Login(loginRequest)).Result – ertan2002 Jan 18 '17 at 13:39
  • So you have a deadlock or are losing tasks. – Lasse V. Karlsen Jan 18 '17 at 13:49
  • Yes I have a deadlock but i dont know because of which object. When I use Wait function instead of await it works.. – ertan2002 Jan 18 '17 at 13:59

2 Answers2

2

Here is a working .NET Fiddle.

I think your .Login method is trying to do too much. The first thing that I noticed (and can only imagine how it's implemented) is the static ClientGenerator, that has static mutable state. This which is alarming and a very specific code smell. I would love to see what the client itself looks like and how that is implemented as that would certainly help to better answer this question.

Based on what you shared thus far (and assuming that the client.loginAsync returns a Task<loginResponse>), I would say that you could do the following:

public class SignServiceWrapper
{
    private static string _webServiceUrl;
    private  BrokerClientClient client;

    public SignServiceWrapper(string webServiceUrl)
    {
        _webServiceUrl = webServiceUrl;
    }

    public Task<loginResponse> LoginAsync(loginRequest request)
    {
        ClientGenerator.WebServiceUrl = _webServiceUrl;
        ClientGenerator.InitializeService();
        client = ClientGenerator.ServiceClient;

        return client.loginAsync(request);
    }

    // ...
}

You could then consume this as such:

public async Task<byte[]> Sign(byte[] documentContent)
{
    var service = new SignServiceWrapper("https://example.com?wsdl");
    var loginResult = await service.LoginAsync(loginRequest);

    //...
}

If the client.loginAsync doesn't return what you're looking for, then you'll need to approach this doing something similar to your current approach. Or if you are locked in to the event-based async pattern, you have other considerations - like whether or not you want to support cancellation, IsBusy, progress, incremental results and if you have the ability to have the event args inherit the System.ComponentModel.AsyncCompletedEventArgs, etc...

One final consideration, if the client.loginAsync is Task returning, even if it doesn't return the loginResponse you need to await it like so:

public async Task<loginResponse> Login(loginRequest request)
{
    var tcs = new TaskCompletionSource<loginResponse>();

    ClientGenerator.WebServiceUrl = _webServiceUrl;
    ClientGenerator.InitializeService();
    client = ClientGenerator.ServiceClient;
    client.loginCompleted += (sender, loginResult) =>
    {
        if (loginResult.Error != null)
            tcs.SetException(loginResult.Error);
        else
            tcs.TrySetResult(loginResult.Result);
    };    

    await client.loginAsync(request);                

    return tcs.Task;
}

Update

After discussion with OP this .NET Fiddle seemed to align with his needs.

Community
  • 1
  • 1
David Pine
  • 23,787
  • 10
  • 79
  • 107
  • Thank you for your answer but loginAsync method is a void method so await does not work! Well I solved the problem by using like var loginResult = Task.Run(() => service.Login(loginRequest)).Result; – ertan2002 Jan 18 '17 at 13:36
  • That is not solving the problem, but rather setting yourself up for another deadlock situation. Do you have control over the `.loginAsync` and that client class? – David Pine Jan 18 '17 at 13:39
  • Yes I know that its not a proper solution :) The loginAsync is a webservice function and i cant access the webservice, because its not our project, then is a external source. – ertan2002 Jan 18 '17 at 13:41
  • And I do not want to write event based things :) I was looking for solving the problem with different way.. – ertan2002 Jan 18 '17 at 13:41
  • Did you see my `.NET Fiddle`, that is the correct way to handle this. – David Pine Jan 18 '17 at 13:43
  • Yes I see it, but i dont want to use an eventhandler because the log has already its own completed event. And Login is not only method in this service and i have to write all methods for my service wrapper.. – ertan2002 Jan 18 '17 at 13:48
  • You are already using an event-handler. If you are taking a dependency on this service, and you are wrapping it... that is what you need to do. You could always encapsulate the logic for the various events in your wrapper such that all the calls you need to wrap are delegated down to a single entry point. This would make your code much more SOLID. – David Pine Jan 18 '17 at 13:50
  • you think that my approach is not good? I mean if i use something like that var loginResult = Task.Run(() => service.Login(loginRequest)).Result; or loginResult.Wait(); instead of await keyword? Do I have to use an eventhandler what you wrote? – ertan2002 Jan 18 '17 at 14:00
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/133459/discussion-between-david-pine-and-ertan2002). – David Pine Jan 18 '17 at 14:03
-1

Change var loginResult = await Task.Run(() =>service.Login(loginRequest)); To var loginResult = await service.Login(loginRequest);

Leonid Malyshev
  • 475
  • 1
  • 6
  • 14
  • 1
    That probably isn't enough since the `Login` method also mixes up asynchronous and non-asynchronous methods. Notice the TCS *and* the `loginAsync` method. The problem could in `ClientGenerator` – Panagiotis Kanavos Jan 18 '17 at 09:13