0

I have this piece of code:

ids.ForEach(async id =>
{
    var grupo = await db.GrupoServicio.FindAsync(id).ConfigureAwait(true);
    if (grupo != null)
    {
        grupo.GrupoServicioEliminadoEn = DateTime.Now;
        n++;
        await Command.AgregaGrupoServicioAsync(db, grupo, true).ConfigureAwait(true);
    }
});

await db.SaveChangesAsync().ConfigureAwait(true);

The system is throwing an exception at the SaveChangesAsync call.

The error is:

A second operation started on this context before a previous asynchronous operation completed. Use 'await' to ensure that any asynchronous operations have completed before calling another method on this context. Any instance members are not guaranteed to be thread safe.

As you see, I am using awaitkeyword.

How can I solve it? Should I replace the ForEach call by a normal foreachloop? Or should I wrap the ForEach call with a Task.Run?

Regards Jaime

Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104
jstuardo
  • 3,901
  • 14
  • 61
  • 136
  • What’s the intention behind using ConfigureAwait(true)? Do the operations really need to have the same synchronization context? – NGambit May 20 '20 at 16:29
  • @NGambit yes.... an instruction after the SaveChangesAsync needs the context. – jstuardo May 20 '20 at 16:34
  • 1
    When replacing that `ForEach` for a normal `foreach`, it works. Strange, isn't it? – jstuardo May 20 '20 at 16:49
  • 1
    Take a look at this: [How can I use Async with ForEach?](https://stackoverflow.com/questions/18667633/how-can-i-use-async-with-foreach) Short answer: it's not possible, because `List.ForEach` is not async-friendly. – Theodor Zoulias May 20 '20 at 17:30
  • 1
    `ConfigureAwait(true)` is the default, so specifying that doesn't actually do anything. – Gabriel Luci May 20 '20 at 21:44
  • @GabrielLuci I know that but I have just answered your questions. ConfigureAwait is not the problem here. I place it only to making the code analysis happy. If I don't include it, I receive several unuseful messages that can hide useful ones unless I fill the code with a lot of exclusions. – jstuardo May 24 '20 at 00:08
  • I was just pointing out that it's unnecessary, not that it had anything to do with your problem. If you have a reason for keeping it in your code, that's your call. – Gabriel Luci May 24 '20 at 04:12

1 Answers1

1

A DbContext can only run one operation at a time. The exception is telling you that you're trying to run a second simultaneous operation using the same DbContext object.

The only way to execute multiple database operations at the same time is to create a new DbContext object for each thread (inside the ForEach). That means you'll have to bring your SaveChanges() inside the ForEach.

There is also no point using async/await inside the ForEach since ForEach won't wait for them to complete. The await keyword actually returns when it acts on an incomplete Task. So as soon as the first await is hit, it returns, and because ForEach is not written to use a returned Task, it will think that iteration is done. You can look at that Theodor's link to see ways of making your own asynchronous ForEach.

But the easiest change would be this (assuming you have a synchronous Command.AgregaGrupoServicio):

ids.ForEach(id =>
{
    var db = new MyDbContext();
    var grupo = db.GrupoServicio.Find(id);
    if (grupo != null)
    {
        grupo.GrupoServicioEliminadoEn = DateTime.Now;
        n++;
        Command.AgregaGrupoServicio(db, grupo, true);
        db.SaveChanges();
    }
});

If this is in an ASP.NET project, you really should avoid using multiple threads at all, since ASP.NET has a limited number of threads per application. If every request creates new threads, you can run into that limit.

Gabriel Luci
  • 38,328
  • 4
  • 55
  • 84
  • That is not a good solution.That will imply to do a commit to the database for every record update. Very bad performance. To solve it, I replaced the ForEach by a normal foreach. – jstuardo May 24 '20 at 00:11
  • Both ways will make an equal number of SQL commands to your database. The only difference is that making only one call to `SaveChanes()` will keep everything in one transaction, but that only matters if you want it to roll back everything in the event of a failure. – Gabriel Luci May 24 '20 at 04:10