0

I try to test this structure but it never works.
I believe I have misunderstood it or lack knowledge in concurrency.

Please light me to the correct direction.

I call a save method but this never saves.

My code :

public async Task<MyObject> GetMyObject_ById(int id)
{
    var responseDto = await this.RestApi.QueryMyObject_ById(id).ConfigureAwait(false);

    if (responseDto != null)
    {
       return this.mapper.Map<MyObject>(responseDto);
    }
    else
    {
       return null;
    }
}

public async Task<MyObject> UpdateMyObject(int id, MyObject updatevalue)
{
    var dto = this.mapper.Map<MyObjectDto>(updatevalue);

    var updateinfo = await this.RestApi.Update(id, dto).ConfigureAwait(false);

    return this.mapper.Map<MyObject>(updateinfo);
}


private void Save()
{    
    foreach (MyObject pb in this.ListToSave)
    {
         this.SaveValue(pb.Id, pb.Status));
    }
}

private async Task<MyObject> SaveValue(int id,string status)
{
     var info = this.GetMyObject_ById(id).Result;

     info.Status = status;

     return await this.RestApi.UpdateMyObject(id, info).ConfigureAwait(false);
}
Mong Zhu
  • 23,309
  • 10
  • 44
  • 76
Loran
  • 772
  • 4
  • 16
  • 38
  • 2
    Is it `Save` an entry method (bind to button event)? You can convert the `Save` method to async type and inovke `SaveValue` using await. Don't call aysnc method in sync context – user1672994 Sep 14 '20 at 08:59
  • 1
    Why are you doing .Result in the first line of SaveValue method? And where do you call Save method? If SaveValue is async you should change return type of Save method to async Task as well `await`ing SaveValue – Ankit Vijay Sep 14 '20 at 08:59
  • 6
    You are calling an async method from a sync context. Don't. You probably added the `.Result` exactly because of this. You shouldn't do that, though. Go async _all the way_. Oh and - do not mistake "async" for "concurrency". – Fildor Sep 14 '20 at 09:00
  • 1
    You are not waiting on the tasks you are launching in your foreach loop. Try adding them to a collection (i.e a List) and after the foreach, call Task.WaitAll(). – Irwene Sep 14 '20 at 09:04
  • @Fildor, Your short commend give me long thinking, If you don't mind can u give some example and bit explain for "do not mistake 'async' for concurrency"?Thank u. – Loran Sep 14 '20 at 10:03
  • Regarding that portion of my comment, this answer may be helpful to you: https://stackoverflow.com/a/4844774/982149 – Fildor Sep 14 '20 at 10:40

1 Answers1

3

Just to summarize the comments. You should replace this.GetMyObject_ById(id).Result; with await this.GetMyObject_ById(id); to avoid blocking. .Result and .Wait() should be avoided since they can cause deadlocks.

You probably want your save method to look like:

private async void Save()
{
    try
    {    
         foreach (MyObject pb in this.ListToSave)
        {
             await this.SaveValue(pb.Id, pb.Status));
        }
     }
     catch(Exception e){ 
     // handle exception
     }
}

or

private async Task Save()
{    
    foreach (MyObject pb in this.ListToSave)
    {
         await this.SaveValue(pb.Id, pb.Status));
    }
}

When using async methods you should always have some way to handle the result, even if the result might be an exception. This also executes each save call after the previous has completed, this is typically not a bad idea since it helps avoid threading issues.

JonasH
  • 28,608
  • 2
  • 10
  • 23