この記事の続き。手こずっているシステムのバグ対応でまた躓いた。
Entity Frameworkなんて使ったことねぇよ。わかんねー
事の発端
とある画面を表示する際、初期表示データを取得するためのDBアクセス時にEntity Frameworkが例外エラーを吐くことがある。
A second operation was started on this context before a previous operation completed. This is usually caused by different threads concurrently using the same instance of DbContext. For more information on how to avoid threading issues with DbContext, see https://go.microsoft.com/fwlink/?linkid=2097913.
at Microsoft.EntityFrameworkCore.Internal.ConcurrencyDetector.EnterCriticalSection()
at Microsoft.EntityFrameworkCore.Query.Internal.SingleQueryingEnumerable`1.AsyncEnumerator.MoveNextAsync()
at Microsoft.EntityFrameworkCore.Query.ShapedQueryCompilingExpressionVisitor.SingleAsync[TSource](IAsyncEnumerable`1 asyncEnumerable, CancellationToken cancellationToken)
at Microsoft.EntityFrameworkCore.Query.ShapedQueryCompilingExpressionVisitor.SingleAsync[TSource](IAsyncEnumerable`1 asyncEnumerable, CancellationToken cancellationToken)
at NameSpace.DataAccess.CustomerMasterAccess.GetItem(String customerCode) in C:\*****.cs:line 74
at NameSpace.DataAccess.MasterAccess.GetCustomerName(String customerCode) in C:\*****.cs:line 125
at NameSpace.Forms.DataInput.SetCustomerName(String customerCode, String customerCode2, String customerCode3) in C:\*****.cs:line 475
at NameSpace.Forms.DataInput.DisplayItems() in C:\*****.cs:line 90
at NameSpace.Forms.DataInput.Initial() in C:\*****.cs:line 71
at System.Threading.Tasks.Task.<>c.<ThrowAsync>b__140_0(Object state)
"A second operation started on this context before a previous operation completed"というエラーメッセージでググってみると、「DbContext はスレッドセーフではないので、スレッド間で使いまわしてはいけない」という文言が見つかる。
つまりこのエラーは複数スレッドから同じDbContextにアクセスしてしまったため発生しているようだ(エラーメッセージに書いている通りやね)
確かに非同期でDbContextを扱っているが
呼び出し元(Form)のロジックを見てみると、確かに非同期処理(async/await)になっている。
// 価格情報を取得
var price = new PriceMasterAccess(GetDbContext(true));
bool priceExists = await price.Exists(customerCode, customerCode2, customerCode3, customerCode4, customerCode5);
// 顧客情報を取得
var master = new MasterAccess(GetDbContext(false));
this.lblCustomerName.Text = await master.GetCustomerName(customerCode); ←【ここで例外が発生している】
this.lblCustomerName2.Text = await master.GetCustomerName2(customerCode, customerCode2);
this.lblCustomerName3.Text = await master.GetCustomerName3(customerCode, customerCode2, customerCode3);
GetDbContext() はFormBaseが持つメソッドで、引数trueの時のみ新たにDbContextインスタンスを作成して変数に保持する作りになっている。
protected GetDbContext(bool newContext) {
if (newContext) this.dbContext = new ThisSystemDbContext();
return this.dbContext;
}
同じ画面の中で複数回DBアクセスするので、初回はGetDbContext(true)で新しいインスタンスを生成して、次以降はGetDbContext(false)で変数に保持したインスタンスを使いまわす。
同じ画面でかつ連続したDBアクセスだから同じDbContextを使い続けてもいいよね、という意図だろう。
問題ないように見えるのだが...
更にエラーメッセージで検索してみると「awaitの付け忘れ」により先の処理完了を待たずに次の処理が走り始めてしまってエラーになる例は多く見つかった。
しかし今回のコードをは都度awaitで処理完了を待っているため、複数スレッドから同時にアクセスが起きることはない...ように見える。
DbContext はスレッドセーフではありません。 スレッド間でコンテキストを共有しないでください。 コンテキスト インスタンスの使用を継続する前に、すべての非同期呼び出しを必ず待機するようにしてください。
DbContext の有効期間、構成、および初期化 - EF Core
Microsoftも待機(await)しろと言っている。だからawaitしてるって。そうはならんやろ!
なっとるやろがい
そうはいっても現実では例外エラーが出ているので、どこかで想定外の動きにが起きていると考える他ない。
例えばこちらの事例では上下行ともawaitしているのに、下行でsecond operation startedエラーが出ている。
var booking = await _unitOfWork.GetRepository<Booking>().FindAsync(model.BookingId);
(中略)
var dateExists = await _unitOfWork.GetRepository<Booking>().All()
.AnyAsync(b => b.User.UserName == booking.User.UserName && b.Date == model.Date);
上行のFindAsync()は遅延読み込みなので、bookingが実際に評価されるのは実際に使われるタイミング。つまり下行のAnyAsync()条件に入ってから。
下行のAnyAsync()が今まさに動いているところで上行のFindAsync()の評価が走り出すためsecond operation startedエラーが出ていると考えられる。
遅延実行を念頭に入れる
同じことがStackOverflowにも投稿されていた。
var selectedOrder = dbContext.Orders.Where(x => x.Id == id).Single();
var relatedOrders = dbContext.Orders.Where(x => x.User.Id == selectedOrder.User.Id).ToList();
こういう場合、明示的に項目を変数に取得してから使えばいいらしい。
var selectedOrder = dbContext.Orders.Where(x => x.Id == id).Single();
var userId = selectedOrder.User.Id;
var relatedOrders = dbContext.Orders.Where(x => x.User.Id == userId).ToList();
awaitしているからといって安心はできない。遅延読み込みの動きも念頭に置いてコードを追わないと...。
DbContextを使いまわさない
この記事では「マルチスレッドアプリ内ではスレッド毎に個別のDbContextインスタンスを使え」と紹介されている。最終手段としてはこうしてしまうのが手っ取り早い気もする。
In a multi-threaded application, you must create and use a separate instance of your DbContext-derived class in each thread.
Managing DbContext the right way with Entity Framework 6: an in-depth guide
やや乱暴な気もするが、StackOverflowのスレッドでも同様の意見はあった。
DBコネクションを張りなおすわけではないので処理コストもそこまでかからないとか。
ServiceLifetimeをTransientに設定する
また、Dependency Injectionとやらを使っている場合はServiceLifetimeをTransientに設定する方法も紹介されている。
これはDbContextインスタンスの有効期間を設定するものらしく、デフォルトはScoped(同一リクエスト中はコンテキストを共有)で、Transientに変えると毎度newするようになるらしいが、DIというものを理解していないため割愛する。
DbContextはDisposeすべきか
あと、実はDbContextはIDisposableという話。
DbContextの使いまわしをやめて都度newするのはいいけど、使い終わった後にDisposeしなくていいの?という疑問が浮かんだ。
いくつかのスレッドを確認した結果では「しなくていい」という意見が多かった。
例えばusingで囲んでIQueryableを取得した場合、IQueryableはIEnumerableと同様に遅延読み込みなので、実際に評価されるタイミングがusing外になってしまう場合が出てくる。
IEnumerable<Test> listT1;
using (Model1 db = new Model1())
{
listT1 = db.Tests;
}
foreach (var a in listT1) //dbが実際に評価されるタイミング(もうDisposeされている)
{
Console.WriteLine(a.value);
}
この場合、既にDisposedされているオブジェクトにアクセスするためObjectDisposedExceptionが発生してしまう。
GCに全てを委ねればいいらしい
また「DbContextは裏で上手いこと管理されてる」から不要だという意見もあった。
Entity Frameworkの中の人に質問した内容がこちらの記事に載っている。
The default behavior of DbContext is that the underlying connection is automatically opened any time is needed and closed when it is no longer needed. E.g. when you execute a query and iterate over query results using "foreach", the call to IEnumerable<T>.GetEnumerator() will cause the connection to be opened, and when later there are no more results available, "foreach" will take care of calling Dispose on the enumerator, which will close the connection. In a similar way, a call to DbContext.SaveChanges() will open the connection before sending changes to the database and will close it before returning.
Given this default behavior, in many real-world cases it is harmless to leave the context without disposing it and just rely on garbage collection.
Do I always have to call Dispose() on my DbContext objects? Nope
接続のオープン/クローズは自動で行われており、多くのケースではDisposeせずにGCに任せていても実害はない、ということらしい。
「じゃあ何でIDisposableなんだよ」という話ではあるが、あくまでもDisposableなのであってmust-disposeではないということで(ちょっと無理があるか)
コメントする