v10.1.0 - Channel-based write queue, BenchmarkDotNet suite, SEO docs refresh, and trust signals - #7
Conversation
This commit fundamentally refactors the internal write serialization mechanism for SQLite from `SemaphoreSlim` to `System.Threading.Channels.Channel`. This change allows for: - Improved write throughput and better resource utilization by processing writes through a single background task per database file. - Configurable back-pressure via the new `WriteQueueCapacity` option, enabling bounded queues to manage write load. - Broader compatibility for the core queue logic by multi-targeting the library to `netstandard2.0`. Additionally, a new test project `EFCore.Sqlite.Concurrency.Test` has been introduced. This project contains comprehensive stress tests that verify the correctness and robustness of all concurrent write operations, including `SaveChangesSerializedAsync`, `BulkInsertOptimizedAsync`, and `ThreadSafeSqliteContext.ExecuteWriteAsync`, as well as mixed read/write scenarios. These tests run dozens of concurrent writers and readers to assert data integrity under heavy load.
Spec covers: problem-first README rewrite, csproj metadata update, QUICKSTART rebrand, four new docs/ microsite pages, and v10.1.0 release notes. Primary SEO target: SQLITE_BUSY / database is locked search terms. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Updated description to lead with SQLITE_BUSY/SQLITE_BUSY_SNAPSHOT error strings for NuGet/AI indexing. Expanded tags with error-5, channel-queue, netstandard2, task-whenall. Release notes updated to v10.1.0. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Leads with exact SQLITE_BUSY exception string. Error table maps each exception to root cause and fix. FAQ adds 4 new entries covering Channel queue, SQLITE_BUSY_SNAPSHOT semantics, netstandard2.0, overhead. Benchmark table relabeled "Typical Results". Links to new docs/ pages. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…oncurrency Removes all references to old package name ThreadSafeEFCore.SQLite. Problem-first opening, two registration patterns only, links to new docs/ microsite pages. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Channel queue, WriteQueueCapacity, netstandard2.0 TFM, bug fixes for ThreadSafeSqliteContext options and BulkInsertSafeAsync O(n^2)/ChangeTracker. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
troubleshooting-sqlite-busy.md — owns SQLITE_BUSY/517/SQLITE_LOCKED search terms concurrent-efcore-patterns.md — owns DbContext thread-safety / Task.WhenAll terms performance-guide.md — owns bulk insert / WAL tuning / Channel queue terms migration-guide.md — owns migration from plain UseSqlite terms Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Runs xUnit stress tests on every push/PR. Publishes TRX results via dorny/test-reporter so failures are visible as PR checks. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…plate CHANGELOG covers v10.0.0 through v10.1.0 in Keep a Changelog format. CONTRIBUTING documents build/test/benchmark commands and PR checklist. SECURITY documents vulnerability reporting process. Issue templates (bug/feature) use GitHub form syntax with structured fields. PR template includes type-of-change, testing, and breaking-change checklist. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Three benchmark classes: - BulkInsertBenchmarks: BulkInsertOptimizedAsync vs SaveChanges-per-entity (1k, 10k records) - ConcurrentWriteBenchmarks: 10 and 50 concurrent SaveChangesSerializedAsync writers - ReadBenchmarks: 20 parallel readers, WAL vs default journal mode Run: cd EFCore.Sqlite.Concurrency.Benchmarks && dotnet run -c Release -- --job short --filter * Full precision: dotnet run -c Release -- --filter * Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds gitignore rules for EFCore.Sqlite.Concurrency.Benchmarks/bin, obj, and BenchmarkDotNet.Artifacts/. Removes previously tracked build artifacts. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…esults BulkInsertOptimizedAsync vs plain SaveChanges-per-entity (1000 entities): Baseline: 4500 ms | Package: 28 ms -> ~161x faster (N=84, BDN v0.14.0) Measured on .NET 10.0.2 / Windows 11 / Intel i7-13700K. Also adds CI badge to README and Running the Benchmarks section to docs/performance-guide.md with reproduce commands. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
SQLitePCLRaw.lib.e_sqlite3 2.1.11 is both the latest available release and the vulnerable one - no patched version exists on NuGet yet. The vulnerability is in the bundled native SQLite binary and comes in as a transitive dependency via Microsoft.Data.Sqlite. Added NuGetAuditSuppress scoped to the exact advisory URL so all other high-severity advisories still surface. Remove suppression once a fixed version ships upstream. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Warning Review limit reached
Next review available in: 45 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThis PR replaces semaphore-based SQLite write serialization with a Channel-based write queue, adds a ChangesCore Write Queue Implementation
Benchmark Suite
Concurrency Stress Test Suite
Documentation, CI, and Community Files
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Estimated code review effort: 4 (Complex) | ~60 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
dorny/test-reporter@v1 uses git ls-files on pull_request events and never sees the runtime-generated .trx file, causing fail-on-empty to kill the build even when all tests pass. The dotnet test exit code already gates the build correctly so the reporter adds no value here. Also adds NuGetAuditSuppress for GHSA-2m69-gcr7-jv3q to the test project csproj to silence the SQLitePCLRaw.lib.e_sqlite3 NU1903 warning that was appearing during test project restore and build.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 12
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/ci.yml (1)
10-48: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winTighten the workflow permissions.
This job inherits the default
GITHUB_TOKENscope, which is broader than this build/test/report flow needs. Add an explicitpermissionsblock so the reporter gets only the access it needs and the rest stays read-only.Suggested change
permissions: contents: read checks: write🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 10 - 48, The build-and-verify workflow currently relies on the default GITHUB_TOKEN permissions, which are broader than needed for the checkout/build/test/report flow. Add an explicit permissions block to the build job in ci.yml so the job is read-only by default and only grants the reporter the access it needs, using the build job and Publish Test Results / dorny/test-reporter@v1 setup as the place to apply it.Source: Linters/SAST tools
🧹 Nitpick comments (7)
docs/superpowers/specs/2026-07-05-trust-signals-design.md (1)
34-35: 🚀 Performance & Scalability | 🔵 TrivialKeep a regression guard for the published benchmark numbers.
If the README is going to advertise real benchmark figures, excluding the benchmark project from CI means those numbers can drift without any automated signal. Consider at least a lightweight scheduled or smoke benchmark so the published results stay attributable to a known commit/environment.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/superpowers/specs/2026-07-05-trust-signals-design.md` around lines 34 - 35, The spec currently excludes the benchmark project from CI, but the published benchmark numbers need a regression guard so they do not drift unnoticed. Update the trust-signals design to add a lightweight scheduled or smoke benchmark path alongside the existing benchmark project, and reference the benchmark project / README benchmark claims so the published figures stay tied to a known commit and environment.EntityFrameworkCore.Sqlite.Concurrency/src/SqliteWriteQueue.cs (1)
44-70: 🩺 Stability & Availability | 🔵 Trivial | 🏗️ Heavy liftNo supervision if the background writer loop dies.
RunAsyncwraps each request's execution in try/finally that only resetsIsWriteLockHeld; nothing observes_writerTaskif the loop itself exits abnormally (e.g., an exception escaping theIsWriteLockHeldassignment orReadAllAsync/TryReaditself). If that ever happens, the channel silently stops being drained — all subsequentEnqueueAsynccalls for that database will enqueue successfully but hang forever awaitingCompletion.Task, with no error surfaced until the process is restarted.Consider observing
_writerTask's exception (e.g., via a continuation that logs/propagates) so a dead writer doesn't manifest as an indefinite, silent hang for every future write to that database.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@EntityFrameworkCore.Sqlite.Concurrency/src/SqliteWriteQueue.cs` around lines 44 - 70, The background writer loop in RunAsync can fail silently, leaving _writerTask faulted while EnqueueAsync keeps waiting forever on Completion.Task. Add supervision for _writerTask when it is created or stored in SqliteWriteQueue, such as a continuation or observer that logs and propagates any exception from RunAsync so a dead writer is surfaced instead of hanging future writes. Make sure the fix references the existing RunAsync, _writerTask, and EnqueueAsync flow so the queue cannot stop draining without notification.EFCore.Sqlite.Concurrency.Test/TempDatabase.cs (1)
25-39: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winFlaky cleanup: replace fixed sleep with deterministic pool clear.
Thread.Sleep(50)is a heuristic wait for SQLite's connection pool to release the file handle. Under the heavy concurrent load these stress tests generate (hundreds of contexts per test), 50ms may not be sufficient, and failures are silently swallowed, potentially leaking locked temp files across CI runs.Microsoft.Data.Sqlite.SqliteConnection.ClearAllPools()deterministically empties the pool before deletion.♻️ Proposed fix
+using Microsoft.Data.Sqlite; + namespace EFCore.Sqlite.Concurrency.Test; ... public void Dispose() { if (_disposed) return; _disposed = true; - // Allow EF Core connection pool to drain before deleting the file. - Thread.Sleep(50); + // Force-release pooled native connections/file locks deterministically. + SqliteConnection.ClearAllPools(); foreach (var ext in new[] { "", "-wal", "-shm" })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@EFCore.Sqlite.Concurrency.Test/TempDatabase.cs` around lines 25 - 39, Replace the heuristic wait in TempDatabase.Dispose with a deterministic SQLite pool cleanup: remove the Thread.Sleep(50) call and invoke Microsoft.Data.Sqlite.SqliteConnection.ClearAllPools() before deleting the temp database files. Keep the existing disposal flow in TempDatabase.Dispose and the file cleanup loop for _dbPath, but ensure the pool is cleared first so locked files are released reliably under heavy concurrency.EFCore.Sqlite.Concurrency.Test/ConcurrencyStressTests.cs (1)
166-186: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winReader assertion is tautological and verifies nothing.
CountAsync()can never return a negative number, soAssert.True(count >= 0)will always pass regardless of correctness. This gives false confidence that concurrent reads were validated; it doesn't actually check for torn reads, exceptions, or monotonic growth as the test name implies.Consider asserting something meaningful, e.g. that every observed count is between 1 (seed row) and
expected + 1, or capturing/asserting no exceptions were thrown by reader tasks.♻️ Proposed fix
- // All reader results must be non-negative (reads always saw valid data) - Assert.All(readResults, count => Assert.True(count >= 0)); + // Every observed count must be a valid snapshot between the seed row and the final total. + Assert.All(readResults, count => Assert.InRange(count, 1, expected + 1));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@EFCore.Sqlite.Concurrency.Test/ConcurrencyStressTests.cs` around lines 166 - 186, The reader check in ConcurrencyStressTests is currently tautological because CountAsync() cannot return a negative value, so update the assertion on readResults to validate something meaningful. In the test that creates readerTasks and uses ctx.Entities.CountAsync(), assert the observed counts stay within the expected range (at least the seed row and at most expected + 1), or otherwise capture and assert reader task failures so the concurrency behavior is actually verified.EFCore.Sqlite.Concurrency.Benchmarks/EFCore.Sqlite.Concurrency.Benchmarks.csproj (1)
5-14: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winVerify BenchmarkDotNet 0.14.0 compatibility with
net10.0.The project multi-targets
net10.0only, but BenchmarkDotNet 0.14.0 predates official .NET 10 support. Per BenchmarkDotNet's release notes, "BenchmarkDotNet v0.15.0 brings .NET 10 support" and explicitly "Added RuntimeMoniker.Net10, NativeAot10, and Mono10 with full toolchain support". The current latest stable release is 0.15.8.Since this benchmark project is meant to produce the numbers published in the README/release notes, running on an unsupported BenchmarkDotNet version risks build/toolchain-resolution failures or unreliable results on net10.0.
♻️ Suggested version bump
- <PackageReference Include="BenchmarkDotNet" Version="0.14.0" /> + <PackageReference Include="BenchmarkDotNet" Version="0.15.8" />Please confirm whether BenchmarkDotNet 0.14.0 fully supports building/running benchmarks on
net10.0, or if it should be bumped to a version with official .NET 10 support.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@EFCore.Sqlite.Concurrency.Benchmarks/EFCore.Sqlite.Concurrency.Benchmarks.csproj` around lines 5 - 14, BenchmarkDotNet 0.14.0 may not fully support the project’s net10.0 target, so update the BenchmarkDotNet PackageReference in EFCore.Sqlite.Concurrency.Benchmarks to a release with official .NET 10 support. Verify the benchmark project’s package version against the library’s release notes, and if needed bump it to a version that includes RuntimeMoniker.Net10 support so the benchmark runner can build and execute reliably on net10.0.EFCore.Sqlite.Concurrency.Benchmarks/BulkInsertBenchmarks.cs (1)
44-78: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winIteration state reset may not be honored under invocation unrolling.
[IterationSetup]clears rows once per iteration, but BenchmarkDotNet may run several invocations ofBaseline_SaveChangesPerEntity/Package_BulkInsertOptimizedAsyncper iteration (unroll factor > 1) if a single invocation completes quickly. In that case, later invocations within the same iteration insert on top of rows left by earlier ones, growing the table and skewing measured timings — undermining the accuracy of numbers meant to be published in the README/docs.Consider pinning
UnrollFactor: 1(e.g. via[SimpleJob(invocationCount: ..., unrollFactor: 1)]orJob.Default.WithUnrollFactor(1)) soIterationSetupreliably runs before each measured invocation. The same risk applies toConcurrentWriteBenchmarks.cs, which uses the identical setup/benchmark pattern.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@EFCore.Sqlite.Concurrency.Benchmarks/BulkInsertBenchmarks.cs` around lines 44 - 78, The iteration reset in IterationSetup can be bypassed when BenchmarkDotNet unrolls multiple invocations per iteration, causing Baseline_SaveChangesPerEntity and Package_BulkInsertOptimizedAsync to accumulate rows and skew results. Fix this by pinning the benchmark job to UnrollFactor = 1 (for example via SimpleJob or a job configuration) so each invocation is preceded by the setup. Apply the same change to the matching benchmark pattern in ConcurrentWriteBenchmarks as well.EntityFrameworkCore.Sqlite.Concurrency/EFCore.Sqlite.Concurrency.csproj (1)
108-117: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRedundant exclusion pattern.
Line 114 (
src\ExtensionMethods\**\*.cs) already coversMemoryPackExtensions.csunder the same folder; Line 116 is a no-op duplicate.🧹 Proposed cleanup
<Compile Remove="src\ExtensionMethods\**\*.cs" /> <Compile Remove="src\SqliteDiagnostics.cs" /> - <Compile Remove="src\ExtensionMethods\MemoryPackExtensions.cs" />🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@EntityFrameworkCore.Sqlite.Concurrency/EFCore.Sqlite.Concurrency.csproj` around lines 108 - 117, The netstandard2.0 ItemGroup in EFCore.Sqlite.Concurrency.csproj contains a redundant Compile Remove pattern: src\ExtensionMethods\**\*.cs already excludes MemoryPackExtensions.cs, so remove the explicit MemoryPackExtensions.cs entry and keep the broader ExtensionMethods exclusion in place.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/concurrent-efcore-patterns.md`:
- Around line 74-80: The concurrent EF Core sample that uses
_factory.CreateDbContext() and FindAsync can dereference a null item when an ID
is missing. Update this snippet to check the result of FindAsync before
accessing item.ProcessedAt, and skip or handle missing records safely. Apply the
same null guard pattern to the equivalent FindAsync examples in the other
documentation snippets so they all behave consistently.
- Around line 153-158: The write-queue description is using the wrong cache key;
it should match the implementation in the write-queue/cache code rather than
saying “database file path.” Update the wording in the concurrency patterns docs
to reference the actual queue key used by the implementation (the connection
string–based cache in the relevant write-queue manager/serializer), and make
sure the “all three patterns share the same queue” statement only claims sharing
when the same implementation key is reused.
In `@docs/migration-guide.md`:
- Around line 103-106: The migration guide bullet overstates the change by
telling users to replace every SaveChangesAsync call with
SaveChangesSerializedAsync, which conflicts with the existing compatibility
guidance. Update the migration-guide entry to frame SaveChangesSerializedAsync
as an optional first-class helper for cases where serialization is needed, and
keep the wording aligned with the SaveChangesSerializedAsync/SaveChangesAsync
guidance elsewhere so existing call sites are not presented as requiring a
mandatory API swap.
In `@docs/superpowers/plans/2026-07-05-v10-1-0-docs-seo.md`:
- Around line 320-326: The out-of-scope block is stale because the companion
trust-signals spec now includes a root CHANGELOG.md and a BenchmarkDotNet
project. Update the relevant section in this docs plan to remove those items
from “out of scope” and align the scope with the current PR objectives, using
the existing plan section that lists platform and dependency requirements as the
anchor.
In `@docs/superpowers/specs/2026-07-05-v10-1-0-docs-seo-design.md`:
- Around line 81-86: Update the EF Core exception row in the docs spec to use
the canonical full message, since the current text is missing the word instance.
In the table entry for the DbContext concurrency issue, keep the exact exception
string under the existing InvalidOperationException example and preserve the
surrounding explanation tied to IDbContextFactory<T>.
- Around line 123-126: The netstandard2.0 compatibility note is incomplete
because it only names EF Core-dependent APIs and omits the registration helpers
that are also net10.0-only. Update the FAQ entry in the docs spec to mention
AddConcurrentSqliteDbContext<T> and AddConcurrentSqliteDbContextFactory<T>
alongside UseSqliteWithConcurrency and ThreadSafeSqliteContext, so the
net10.0-only list matches the actual API surface.
In
`@EntityFrameworkCore.Sqlite.Concurrency/src/Models/SqliteConcurrencyOptions.cs`:
- Around line 115-136: Add range validation for WriteQueueCapacity in
SqliteConcurrencyOptions.Validate() so invalid values fail fast like
MaxRetryAttempts, CommandTimeout, and WalAutoCheckpoint. Reject 0 and negative
values with an ArgumentOutOfRangeException that clearly names
WriteQueueCapacity, and keep the check in the Validate() method so the error is
raised before SqliteWriteQueue or Channel.CreateBounded is reached.
In `@EntityFrameworkCore.Sqlite.Concurrency/src/SqliteConnectionEnhancer.cs`:
- Around line 63-66: Serialize the initialization in GetWriteQueue because
ConcurrentDictionary.GetOrAdd can invoke the factory multiple times and create
extra SqliteWriteQueue instances with live writer tasks. Update
SqliteConnectionEnhancer.GetWriteQueue to use a single-initialization guard such
as Lazy<SqliteWriteQueue> keyed by connection string, and ensure the first
access creates only one SqliteWriteQueue and one _writerTask per connection
string.
In `@EntityFrameworkCore.Sqlite.Concurrency/src/SqliteWriteQueue.cs`:
- Around line 28-42: The cancellation behavior in EnqueueAsync<T> is
inconsistent across TFMs because WriteAsync only controls queue admission, while
WaitAsync(ct) can cancel after the request is already enqueued on
non-NETSTANDARD2_0. Update SqliteWriteQueue.EnqueueAsync<T> so post-enqueue
cancellation does not make callers think the write was aborted when the queued
work still runs, either by awaiting WriteRequest<T>.Completion.Task
unconditionally after _channel.Writer.WriteAsync(request, ct) or by explicitly
documenting and aligning the split semantics between NETSTANDARD2_0 and other
targets.
In `@EntityFrameworkCore.Sqlite.Concurrency/src/ThreadSafeSqliteContext.cs`:
- Around line 125-128: The retry delay calculation in ThreadSafeSqliteContext is
off by one: the first retry currently uses 200 ms because attempt is incremented
before the backoff is computed. Update the exponential backoff formula to use
attempt - 1 in the exponent so the first delay matches the
SqliteConcurrencyOptions.MaxRetryAttempts documentation at 100 ms. Also fix the
inline comment near the Task.Delay call to describe the actual jitter behavior,
since the current “full jitter” wording does not match the sampled range.
- Around line 90-131: The retry loop in
ThreadSafeSqliteContext.WriteQueue.EnqueueAsync reuses the same DbContext after
a SQLITE_BUSY catch, so tracked Added/Modified entities can be replayed on the
next attempt. Clear the DbContext ChangeTracker before retrying inside the
busy-exception path, ideally right after catching SqliteException and before the
backoff delay or next loop iteration, so each operation((TContext)(object)this)
attempt starts from a clean state.
In `@README.md`:
- Around line 128-133: The retry sample using ExecuteWithRetryAsync only adds
records to ctx.Records and never persists them, so update this example to either
call SaveChangesAsync inside the ExecuteWithRetryAsync lambda or replace it with
the auto-committing helper used elsewhere in the README. Use the
ExecuteWithRetryAsync snippet to locate the example and make sure the documented
flow actually writes the staged entities.
---
Outside diff comments:
In @.github/workflows/ci.yml:
- Around line 10-48: The build-and-verify workflow currently relies on the
default GITHUB_TOKEN permissions, which are broader than needed for the
checkout/build/test/report flow. Add an explicit permissions block to the build
job in ci.yml so the job is read-only by default and only grants the reporter
the access it needs, using the build job and Publish Test Results /
dorny/test-reporter@v1 setup as the place to apply it.
---
Nitpick comments:
In `@docs/superpowers/specs/2026-07-05-trust-signals-design.md`:
- Around line 34-35: The spec currently excludes the benchmark project from CI,
but the published benchmark numbers need a regression guard so they do not drift
unnoticed. Update the trust-signals design to add a lightweight scheduled or
smoke benchmark path alongside the existing benchmark project, and reference the
benchmark project / README benchmark claims so the published figures stay tied
to a known commit and environment.
In `@EFCore.Sqlite.Concurrency.Benchmarks/BulkInsertBenchmarks.cs`:
- Around line 44-78: The iteration reset in IterationSetup can be bypassed when
BenchmarkDotNet unrolls multiple invocations per iteration, causing
Baseline_SaveChangesPerEntity and Package_BulkInsertOptimizedAsync to accumulate
rows and skew results. Fix this by pinning the benchmark job to UnrollFactor = 1
(for example via SimpleJob or a job configuration) so each invocation is
preceded by the setup. Apply the same change to the matching benchmark pattern
in ConcurrentWriteBenchmarks as well.
In
`@EFCore.Sqlite.Concurrency.Benchmarks/EFCore.Sqlite.Concurrency.Benchmarks.csproj`:
- Around line 5-14: BenchmarkDotNet 0.14.0 may not fully support the project’s
net10.0 target, so update the BenchmarkDotNet PackageReference in
EFCore.Sqlite.Concurrency.Benchmarks to a release with official .NET 10 support.
Verify the benchmark project’s package version against the library’s release
notes, and if needed bump it to a version that includes RuntimeMoniker.Net10
support so the benchmark runner can build and execute reliably on net10.0.
In `@EFCore.Sqlite.Concurrency.Test/ConcurrencyStressTests.cs`:
- Around line 166-186: The reader check in ConcurrencyStressTests is currently
tautological because CountAsync() cannot return a negative value, so update the
assertion on readResults to validate something meaningful. In the test that
creates readerTasks and uses ctx.Entities.CountAsync(), assert the observed
counts stay within the expected range (at least the seed row and at most
expected + 1), or otherwise capture and assert reader task failures so the
concurrency behavior is actually verified.
In `@EFCore.Sqlite.Concurrency.Test/TempDatabase.cs`:
- Around line 25-39: Replace the heuristic wait in TempDatabase.Dispose with a
deterministic SQLite pool cleanup: remove the Thread.Sleep(50) call and invoke
Microsoft.Data.Sqlite.SqliteConnection.ClearAllPools() before deleting the temp
database files. Keep the existing disposal flow in TempDatabase.Dispose and the
file cleanup loop for _dbPath, but ensure the pool is cleared first so locked
files are released reliably under heavy concurrency.
In `@EntityFrameworkCore.Sqlite.Concurrency/EFCore.Sqlite.Concurrency.csproj`:
- Around line 108-117: The netstandard2.0 ItemGroup in
EFCore.Sqlite.Concurrency.csproj contains a redundant Compile Remove pattern:
src\ExtensionMethods\**\*.cs already excludes MemoryPackExtensions.cs, so remove
the explicit MemoryPackExtensions.cs entry and keep the broader ExtensionMethods
exclusion in place.
In `@EntityFrameworkCore.Sqlite.Concurrency/src/SqliteWriteQueue.cs`:
- Around line 44-70: The background writer loop in RunAsync can fail silently,
leaving _writerTask faulted while EnqueueAsync keeps waiting forever on
Completion.Task. Add supervision for _writerTask when it is created or stored in
SqliteWriteQueue, such as a continuation or observer that logs and propagates
any exception from RunAsync so a dead writer is surfaced instead of hanging
future writes. Make sure the fix references the existing RunAsync, _writerTask,
and EnqueueAsync flow so the queue cannot stop draining without notification.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 46599b75-bd85-4a92-ae33-94456a937d04
📒 Files selected for processing (45)
.github/ISSUE_TEMPLATE/bug_report.yml.github/ISSUE_TEMPLATE/feature_request.yml.github/PULL_REQUEST_TEMPLATE.md.github/workflows/ci.yml.gitignoreCHANGELOG.mdCONTRIBUTING.mdEFCore.Sqlite.Concurrency.Benchmarks/BaselineSqliteContext.csEFCore.Sqlite.Concurrency.Benchmarks/BenchmarkEntity.csEFCore.Sqlite.Concurrency.Benchmarks/BulkInsertBenchmarks.csEFCore.Sqlite.Concurrency.Benchmarks/ConcurrentSqliteContext.csEFCore.Sqlite.Concurrency.Benchmarks/ConcurrentWriteBenchmarks.csEFCore.Sqlite.Concurrency.Benchmarks/EFCore.Sqlite.Concurrency.Benchmarks.csprojEFCore.Sqlite.Concurrency.Benchmarks/Program.csEFCore.Sqlite.Concurrency.Benchmarks/ReadBenchmarks.csEFCore.Sqlite.Concurrency.Test/ConcurrencyStressTests.csEFCore.Sqlite.Concurrency.Test/EFCore.Sqlite.Concurrency.Test.csprojEFCore.Sqlite.Concurrency.Test/StressDbContext.csEFCore.Sqlite.Concurrency.Test/StressEntity.csEFCore.Sqlite.Concurrency.Test/TempDatabase.csEFCore.Sqlite.Concurrency.Test/ThreadSafeSqliteStressContext.csEntityFrameworkCore.Sqlite.Concurrency.slnEntityFrameworkCore.Sqlite.Concurrency.sln.DotSettings.userEntityFrameworkCore.Sqlite.Concurrency/EFCore.Sqlite.Concurrency.csprojEntityFrameworkCore.Sqlite.Concurrency/out/EntityFrameworkCore.Sqlite.Concurrency.10.0.4.nupkgEntityFrameworkCore.Sqlite.Concurrency/out/EntityFrameworkCore.Sqlite.Concurrency.10.0.4.snupkgEntityFrameworkCore.Sqlite.Concurrency/packages.lock.jsonEntityFrameworkCore.Sqlite.Concurrency/src/Models/SqliteConcurrencyOptions.csEntityFrameworkCore.Sqlite.Concurrency/src/Polyfills.csEntityFrameworkCore.Sqlite.Concurrency/src/SqliteConcurrencyExtensions.csEntityFrameworkCore.Sqlite.Concurrency/src/SqliteConcurrencyInterceptor.csEntityFrameworkCore.Sqlite.Concurrency/src/SqliteConnectionEnhancer.csEntityFrameworkCore.Sqlite.Concurrency/src/SqliteWriteQueue.csEntityFrameworkCore.Sqlite.Concurrency/src/ThreadSafeSqliteContext.csREADME.mdSECURITY.mddoc/QUICKSTART.mddoc/v10_1_0.mddocs/concurrent-efcore-patterns.mddocs/migration-guide.mddocs/performance-guide.mddocs/superpowers/plans/2026-07-05-v10-1-0-docs-seo.mddocs/superpowers/specs/2026-07-05-trust-signals-design.mddocs/superpowers/specs/2026-07-05-v10-1-0-docs-seo-design.mddocs/troubleshooting-sqlite-busy.md
| var tasks = itemIds.Select(async id => | ||
| { | ||
| // Each task creates and disposes its own context | ||
| await using var db = _factory.CreateDbContext(); | ||
| var item = await db.Items.FindAsync(id, ct); | ||
| item.ProcessedAt = DateTime.UtcNow; | ||
| await db.SaveChangesAsync(ct); // writes serialized by the write queue |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Guard FindAsync before dereferencing.
FindAsync can return null, so this sample can throw on a missing ID. The same guard should be applied to the similar snippets in README.md and doc/QUICKSTART.md.
Proposed fix
var item = await db.Items.FindAsync(id, ct);
+if (item is null)
+ continue;
item.ProcessedAt = DateTime.UtcNow;🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/concurrent-efcore-patterns.md` around lines 74 - 80, The concurrent EF
Core sample that uses _factory.CreateDbContext() and FindAsync can dereference a
null item when an ID is missing. Update this snippet to check the result of
FindAsync before accessing item.ProcessedAt, and skip or handle missing records
safely. Apply the same null guard pattern to the equivalent FindAsync examples
in the other documentation snippets so they all behave consistently.
| ## How the Write Queue Serializes Across All Patterns | ||
|
|
||
| All three patterns share the same process-wide write queue (keyed by database file path). | ||
| When any of them calls a write API (`SaveChangesAsync`, `SaveChangesSerializedAsync`, | ||
| `BulkInsertOptimizedAsync`, `ExecuteWriteAsync`), the work is enqueued to the same | ||
| `Channel<IWriteRequest>`. The single background writer executes one request at a time. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Match the queue key to the implementation.
As per the stack summary, the write-queue cache is per connection string, not per database file path. This wording suggests two equivalent connection strings will always share a queue, which may not hold.
Proposed fix
-All three patterns share the same process-wide write queue (keyed by database file path).
+All three patterns share the same process-wide write queue (cached per connection string).📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ## How the Write Queue Serializes Across All Patterns | |
| All three patterns share the same process-wide write queue (keyed by database file path). | |
| When any of them calls a write API (`SaveChangesAsync`, `SaveChangesSerializedAsync`, | |
| `BulkInsertOptimizedAsync`, `ExecuteWriteAsync`), the work is enqueued to the same | |
| `Channel<IWriteRequest>`. The single background writer executes one request at a time. | |
| ## How the Write Queue Serializes Across All Patterns | |
| All three patterns share the same process-wide write queue (cached per connection string). | |
| When any of them calls a write API (`SaveChangesAsync`, `SaveChangesSerializedAsync`, | |
| `BulkInsertOptimizedAsync`, `ExecuteWriteAsync`), the work is enqueued to the same | |
| `Channel<IWriteRequest>`. The single background writer executes one request at a time. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/concurrent-efcore-patterns.md` around lines 153 - 158, The write-queue
description is using the wrong cache key; it should match the implementation in
the write-queue/cache code rather than saying “database file path.” Update the
wording in the concurrency patterns docs to reference the actual queue key used
by the implementation (the connection string–based cache in the relevant
write-queue manager/serializer), and make sure the “all three patterns share the
same queue” statement only claims sharing when the same implementation key is
reused.
| - [ ] **Remove any `SaveChangesSerializedAsync` workarounds** (if you had a custom version). | ||
| `SaveChangesSerializedAsync` is a first-class extension method on `DbContext` — call it | ||
| directly anywhere you previously called `SaveChangesAsync`. | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Don't make SaveChangesSerializedAsync the default migration step.
The README and release notes say existing call sites keep working, but this bullet tells users to replace every SaveChangesAsync call. That turns an optional helper into a mandatory API change.
Proposed fix
- `SaveChangesSerializedAsync` is a first-class extension method on `DbContext` — call it directly anywhere you previously called `SaveChangesAsync`.
+ `SaveChangesSerializedAsync` remains available for code paths that explicitly want serialized saves; existing `SaveChangesAsync` call sites do not need to change.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - [ ] **Remove any `SaveChangesSerializedAsync` workarounds** (if you had a custom version). | |
| `SaveChangesSerializedAsync` is a first-class extension method on `DbContext` — call it | |
| directly anywhere you previously called `SaveChangesAsync`. | |
| - [ ] **Remove any `SaveChangesSerializedAsync` workarounds** (if you had a custom version). | |
| `SaveChangesSerializedAsync` remains available for code paths that explicitly want serialized saves; existing `SaveChangesAsync` call sites do not need to change. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/migration-guide.md` around lines 103 - 106, The migration guide bullet
overstates the change by telling users to replace every SaveChangesAsync call
with SaveChangesSerializedAsync, which conflicts with the existing compatibility
guidance. Update the migration-guide entry to frame SaveChangesSerializedAsync
as an optional first-class helper for cases where serialization is needed, and
keep the wording aligned with the SaveChangesSerializedAsync/SaveChangesAsync
guidance elsewhere so existing call sites are not presented as requiring a
mandatory API swap.
|
|
||
| - .NET 10.0+ (for EF Core APIs) or netstandard 2.0+ (for connection layer only) | ||
| - Entity Framework Core 10.0+ | ||
| - Microsoft.Data.Sqlite 10.0+ | ||
| - SQLite 3.35.0+ (WAL2 and `SQLITE_BUSY_SNAPSHOT` require 3.37.0+) | ||
|
|
||
| ## License |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Update the out-of-scope block.
This note is stale: the companion trust-signals spec and the PR objectives now add a root CHANGELOG.md and a BenchmarkDotNet project. Leaving both items marked “out of scope” will send implementers in the wrong direction.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/plans/2026-07-05-v10-1-0-docs-seo.md` around lines 320 -
326, The out-of-scope block is stale because the companion trust-signals spec
now includes a root CHANGELOG.md and a BenchmarkDotNet project. Update the
relevant section in this docs plan to remove those items from “out of scope” and
align the scope with the current PR objectives, using the existing plan section
that lists platform and dependency requirements as the anchor.
| | Exception / Error | Root Cause | What This Package Does | | ||
| |---|---|---| | ||
| | `SQLite Error 5: 'database is locked'` (`SQLITE_BUSY`) | Multiple writers contending simultaneously | Channel-based write queue — one writer at a time, all others park efficiently | | ||
| | `SQLite Error 5: 'database is locked'` (extended code 517, `SQLITE_BUSY_SNAPSHOT`) | WAL read snapshot became stale mid-transaction | `BEGIN IMMEDIATE` upgrade + full operation restart on snapshot staleness | | ||
| | `InvalidOperationException: A second operation was started on this context` | `DbContext` shared across concurrent tasks | `IDbContextFactory<T>` registration pattern, one context per concurrent flow | | ||
| | Bulk inserts slow / OOM on large datasets | Linear `SaveChanges()`, unbounded ChangeTracker growth | `BulkInsertOptimizedAsync` — batched, WAL-optimized, ChangeTracker cleared per batch | |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n 'A second operation was started on this context( instance)?' .Repository: CornerstoneCode/EntityFrameworkCore.Sqlite.Concurrency
Length of output: 1421
Use the full EF Core exception string here.
InvalidOperationException: A second operation was started on this context instance is the canonical message, so keep instance in the spec.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/superpowers/specs/2026-07-05-v10-1-0-docs-seo-design.md` around lines 81
- 86, Update the EF Core exception row in the docs spec to use the canonical
full message, since the current text is missing the word instance. In the table
entry for the DbContext concurrency issue, keep the exact exception string under
the existing InvalidOperationException example and preserve the surrounding
explanation tied to IDbContextFactory<T>.
| internal static SqliteWriteQueue GetWriteQueue(string connectionString, int? capacity = null) | ||
| { | ||
| return _writeQueues.GetOrAdd(connectionString, _ => new SqliteWriteQueue(capacity)); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Confirm SqliteWriteQueue starts a background task in its constructor (leak surface).
fd -t f 'SqliteWriteQueue.cs' --exec sed -n '1,40p' {}Repository: CornerstoneCode/EntityFrameworkCore.Sqlite.Concurrency
Length of output: 1580
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the write-queue storage, retrieval, and disposal paths.
FILE='EntityFrameworkCore.Sqlite.Concurrency/src/SqliteConnectionEnhancer.cs'
echo '--- outline ---'
ast-grep outline "$FILE" --view expanded || true
echo
echo '--- relevant ranges ---'
sed -n '1,140p' "$FILE"
echo
sed -n '400,560p' "$FILE"Repository: CornerstoneCode/EntityFrameworkCore.Sqlite.Concurrency
Length of output: 12659
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect SqliteWriteQueue disposal behavior and any cleanup of orphaned queues.
FILE='EntityFrameworkCore.Sqlite.Concurrency/src/SqliteWriteQueue.cs'
echo '--- outline ---'
ast-grep outline "$FILE" --view expanded || true
echo
echo '--- full file sections ---'
sed -n '1,220p' "$FILE"Repository: CornerstoneCode/EntityFrameworkCore.Sqlite.Concurrency
Length of output: 3221
Serialize SqliteWriteQueue creation
ConcurrentDictionary.GetOrAdd may run the factory more than once on concurrent first access. That can start extra SqliteWriteQueue instances whose _writerTask keeps running even though the instance is never stored or disposed. Wrap the queue in Lazy<SqliteWriteQueue> (or another single-initialization guard) so only one writer loop is created per connection string.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@EntityFrameworkCore.Sqlite.Concurrency/src/SqliteConnectionEnhancer.cs`
around lines 63 - 66, Serialize the initialization in GetWriteQueue because
ConcurrentDictionary.GetOrAdd can invoke the factory multiple times and create
extra SqliteWriteQueue instances with live writer tasks. Update
SqliteConnectionEnhancer.GetWriteQueue to use a single-initialization guard such
as Lazy<SqliteWriteQueue> keyed by connection string, and ensure the first
access creates only one SqliteWriteQueue and one _writerTask per connection
string.
| internal async Task<T> EnqueueAsync<T>(Func<Task<T>> work, CancellationToken ct) | ||
| { | ||
| // Reentrancy: already inside the writer loop's async context — execute directly | ||
| // to prevent deadlock from re-queuing behind ourselves. | ||
| if (SqliteConnectionEnhancer.IsWriteLockHeld.Value) | ||
| return await work(); | ||
|
|
||
| var request = new WriteRequest<T>(work); | ||
| await _channel.Writer.WriteAsync(request, ct); | ||
| #if NETSTANDARD2_0 | ||
| return await request.Completion.Task; | ||
| #else | ||
| return await request.Completion.Task.WaitAsync(ct); | ||
| #endif | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the queue implementation and related types/usages.
git ls-files | rg 'EntityFrameworkCore\.Sqlite\.Concurrency/src/|SqliteWriteQueue|WriteRequest|SqliteConnectionEnhancer'
# Show the file with line numbers, but only a bounded slice if it's small enough.
file="EntityFrameworkCore.Sqlite.Concurrency/src/SqliteWriteQueue.cs"
if [ -f "$file" ]; then
echo "=== $file ==="
wc -l "$file"
sed -n '1,220p' "$file" | cat -n
fi
# Find where Completion is completed and how cancellation is handled.
rg -n "Completion|SetResult|SetException|TrySet|WriteRequest<|EnqueueAsync<|IsWriteLockHeld" EntityFrameworkCore.Sqlite.Concurrency/src
# Inspect the write request type and writer loop if present.
for f in $(git ls-files 'EntityFrameworkCore.Sqlite.Concurrency/src/*'); do
case "$f" in
*WriteRequest*|*WriteQueue*|*Enhancer*|*Writer* )
echo "=== $f ==="
wc -l "$f"
sed -n '1,260p' "$f" | cat -n
;;
esac
doneRepository: CornerstoneCode/EntityFrameworkCore.Sqlite.Concurrency
Length of output: 25062
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect callers of EnqueueAsync and any cancellation-related docs/tests.
rg -n "EnqueueAsync<|EnqueueAsync\(" EntityFrameworkCore.Sqlite.Concurrency/src EntityFrameworkCore.Sqlite.Concurrency/test . -g '!**/bin/**' -g '!**/obj/**'
# Read the relevant caller sections.
for f in EntityFrameworkCore.Sqlite.Concurrency/src/SqliteConcurrencyExtensions.cs EntityFrameworkCore.Sqlite.Concurrency/src/SqliteConcurrencyInterceptor.cs; do
if [ -f "$f" ]; then
echo "=== $f ==="
wc -l "$f"
sed -n '120,260p' "$f" | cat -n
fi
done
# Search for cancellation semantics in docs or comments.
rg -n "cancellation|Canceled|OperationCanceledException|WaitAsync\(ct\)|CancellationToken" EntityFrameworkCore.Sqlite.Concurrency/src EntityFrameworkCore.Sqlite.Concurrency/test README.mdRepository: CornerstoneCode/EntityFrameworkCore.Sqlite.Concurrency
Length of output: 1352
🏁 Script executed:
#!/bin/bash
set -euo pipefail
for f in EntityFrameworkCore.Sqlite.Concurrency/src/ThreadSafeSqliteContext.cs EntityFrameworkCore.Sqlite.Concurrency/src/SqliteConcurrencyExtensions.cs; do
if [ -f "$f" ]; then
echo "=== $f ==="
wc -l "$f"
sed -n '70,240p' "$f" | cat -n
fi
doneRepository: CornerstoneCode/EntityFrameworkCore.Sqlite.Concurrency
Length of output: 15756
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== ThreadSafeSqliteContext.cs ==="
sed -n '1,140p' EntityFrameworkCore.Sqlite.Concurrency/src/ThreadSafeSqliteContext.cs | cat -n
echo "=== SqliteConcurrencyExtensions.cs ==="
sed -n '130,240p' EntityFrameworkCore.Sqlite.Concurrency/src/SqliteConcurrencyExtensions.cs | cat -n
echo "=== WaitAsync(ct) usage ==="
rg -n "WaitAsync\(ct\)" EntityFrameworkCore.Sqlite.Concurrency/srcRepository: CornerstoneCode/EntityFrameworkCore.Sqlite.Concurrency
Length of output: 12507
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '145,225p' EntityFrameworkCore.Sqlite.Concurrency/src/SqliteConcurrencyExtensions.cs | cat -n
sed -n '1,130p' EntityFrameworkCore.Sqlite.Concurrency/src/ThreadSafeSqliteContext.cs | cat -nRepository: CornerstoneCode/EntityFrameworkCore.Sqlite.Concurrency
Length of output: 10857
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== SqliteConcurrencyExtensions.cs (145-225) ==="
sed -n '145,225p' EntityFrameworkCore.Sqlite.Concurrency/src/SqliteConcurrencyExtensions.cs | cat -n
echo "=== ThreadSafeSqliteContext.cs (1-130) ==="
sed -n '1,130p' EntityFrameworkCore.Sqlite.Concurrency/src/ThreadSafeSqliteContext.cs | cat -nRepository: CornerstoneCode/EntityFrameworkCore.Sqlite.Concurrency
Length of output: 10949
Don’t let post-enqueue cancellation hide a write WriteAsync(request, ct) only gates admission. On !NETSTANDARD2_0, WaitAsync(ct) can throw after the item is already queued, while the writer still runs it; NETSTANDARD2_0 always waits for completion. That makes cancellation semantics differ by TFM and can let callers treat a write as aborted when it may still commit. If cancellation should stop only queueing, wait on Completion.Task unconditionally or document the split behavior.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@EntityFrameworkCore.Sqlite.Concurrency/src/SqliteWriteQueue.cs` around lines
28 - 42, The cancellation behavior in EnqueueAsync<T> is inconsistent across
TFMs because WriteAsync only controls queue admission, while WaitAsync(ct) can
cancel after the request is already enqueued on non-NETSTANDARD2_0. Update
SqliteWriteQueue.EnqueueAsync<T> so post-enqueue cancellation does not make
callers think the write was aborted when the queued work still runs, either by
awaiting WriteRequest<T>.Completion.Task unconditionally after
_channel.Writer.WriteAsync(request, ct) or by explicitly documenting and
aligning the split semantics between NETSTANDARD2_0 and other targets.
| return await WriteQueue.EnqueueAsync(async () => | ||
| { | ||
| await WriteLock.WaitAsync(ct); | ||
| SqliteConnectionEnhancer.IsWriteLockHeld.Value = true; | ||
|
|
||
| try | ||
| { | ||
| // The interceptor will upgrade this BEGIN to BEGIN IMMEDIATE, ensuring | ||
| // no later statement in the transaction fails with SQLITE_BUSY before | ||
| // commit (as long as UpgradeTransactionsToImmediate is true). | ||
| await using var transaction = await Database.BeginTransactionAsync( | ||
| System.Data.IsolationLevel.Serializable, ct); | ||
|
|
||
| var result = await operation((TContext)(object)this); | ||
| await SaveChangesAsync(ct); | ||
| await transaction.CommitAsync(ct); | ||
|
|
||
| return result; | ||
| } | ||
| catch (SqliteException ex) when (SqliteErrorCodes.IsAnyBusy(ex)) | ||
| int attempt = 0; | ||
| while (true) | ||
| { | ||
| // Release the lock before sleeping so other writers can make progress. | ||
| SqliteConnectionEnhancer.IsWriteLockHeld.Value = false; | ||
| WriteLock.Release(); | ||
|
|
||
| attempt++; | ||
| if (attempt >= maxRetryAttempts) | ||
| try | ||
| { | ||
| var kind = SqliteErrorCodes.IsBusySnapshot(ex) | ||
| ? "SQLITE_BUSY_SNAPSHOT (stale read snapshot — another writer committed after this transaction began)" | ||
| : $"SQLITE_BUSY (extended code {ex.SqliteExtendedErrorCode})"; | ||
|
|
||
| throw new TimeoutException( | ||
| $"SQLite database busy after {attempt} retry attempt(s). " + | ||
| $"Error: {kind}. " + | ||
| $"Consider increasing MaxRetryAttempts or BusyTimeout.", | ||
| ex); | ||
| } | ||
| // The interceptor will upgrade this BEGIN to BEGIN IMMEDIATE, ensuring | ||
| // no later statement in the transaction fails with SQLITE_BUSY before | ||
| // commit (as long as UpgradeTransactionsToImmediate is true). | ||
| await using var transaction = await Database.BeginTransactionAsync( | ||
| System.Data.IsolationLevel.Serializable, ct); | ||
|
|
||
| // Exponential backoff with full jitter: sleep in [baseDelay, 2×baseDelay]. | ||
| // Jitter prevents synchronized retry storms when multiple threads contend. | ||
| var baseDelay = 100 * Math.Pow(2, attempt); | ||
| var jitter = Random.Shared.NextDouble() * baseDelay; | ||
| await Task.Delay(TimeSpan.FromMilliseconds(baseDelay + jitter), ct); | ||
| var result = await operation((TContext)(object)this); | ||
| await SaveChangesAsync(ct); | ||
| await transaction.CommitAsync(ct); | ||
|
|
||
| // Continue to next loop iteration — for BUSY_SNAPSHOT this correctly | ||
| // restarts the entire operation lambda so stale data is re-queried. | ||
| } | ||
| finally | ||
| { | ||
| if (SqliteConnectionEnhancer.IsWriteLockHeld.Value) | ||
| return result; | ||
| } | ||
| catch (SqliteException ex) when (SqliteErrorCodes.IsAnyBusy(ex)) | ||
| { | ||
| SqliteConnectionEnhancer.IsWriteLockHeld.Value = false; | ||
| WriteLock.Release(); | ||
| attempt++; | ||
| if (attempt >= maxRetryAttempts) | ||
| { | ||
| var kind = SqliteErrorCodes.IsBusySnapshot(ex) | ||
| ? "SQLITE_BUSY_SNAPSHOT (stale read snapshot — another writer committed after this transaction began)" | ||
| : $"SQLITE_BUSY (extended code {ex.SqliteExtendedErrorCode})"; | ||
|
|
||
| throw new TimeoutException( | ||
| $"SQLite database busy after {attempt} retry attempt(s). " + | ||
| $"Error: {kind}. " + | ||
| $"Consider increasing MaxRetryAttempts or BusyTimeout.", | ||
| ex); | ||
| } | ||
|
|
||
| // Exponential backoff with full jitter: sleep in [baseDelay, 2×baseDelay]. | ||
| var baseDelay = 100 * Math.Pow(2, attempt); | ||
| var jitter = Random.Shared.NextDouble() * baseDelay; | ||
| await Task.Delay(TimeSpan.FromMilliseconds(baseDelay + jitter), ct); | ||
| } | ||
| } | ||
| } | ||
| }, ct); |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target file with line numbers
sed -n '1,260p' EntityFrameworkCore.Sqlite.Concurrency/src/ThreadSafeSqliteContext.cs | cat -n
# Find any other ChangeTracker.Clear / retry-related logic in the repo
rg -n "ChangeTracker\.Clear|ExecuteWriteAsync|maxRetryAttempts|BUSY_SNAPSHOT|SQLite busy|BusyTimeout|UpgradeTransactionsToImmediate" EntityFrameworkCore.Sqlite.Concurrency -S
# Inspect the interceptor/queue code paths that may affect retry semantics
rg -n "WriteQueue|connectionString|EnqueueAsync|BeginTransactionAsync|SaveChangesAsync" EntityFrameworkCore.Sqlite.Concurrency/src -SRepository: CornerstoneCode/EntityFrameworkCore.Sqlite.Concurrency
Length of output: 32694
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the write-helper methods that implement similar retry/transaction logic
sed -n '140,240p' EntityFrameworkCore.Sqlite.Concurrency/src/SqliteConcurrencyExtensions.cs | cat -n
# Inspect the interceptor and queue implementation around write serialization
sed -n '1,220p' EntityFrameworkCore.Sqlite.Concurrency/src/SqliteConcurrencyInterceptor.cs | cat -n
sed -n '1,220p' EntityFrameworkCore.Sqlite.Concurrency/src/SqliteWriteQueue.cs | cat -n
# Look for any explicit tracker reset in retry paths
rg -n "ChangeTracker\.Clear\(\)|catch \(SqliteException|BUSY_SNAPSHOT|restart the operation|retry" EntityFrameworkCore.Sqlite.Concurrency/src/SqliteConcurrencyExtensions.cs EntityFrameworkCore.Sqlite.Concurrency/src/ThreadSafeSqliteContext.cs EntityFrameworkCore.Sqlite.Concurrency/src/SqliteConcurrencyInterceptor.cs -SRepository: CornerstoneCode/EntityFrameworkCore.Sqlite.Concurrency
Length of output: 23318
Reset ChangeTracker before retrying busy writes. operation runs again on the same DbContext after rollback, but tracked Added/Modified entities remain in the change tracker. That can replay inserts/updates or leave stale state on the next attempt; clear the tracker before the loop retries.
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 126-126: System.Random is a non-cryptographic, predictable PRNG and must not be used to generate security-sensitive values such as tokens, session IDs, passwords, salts, nonces, or API keys. An attacker who observes a few outputs can recover the seed and predict all future values. Use a cryptographically secure source instead: RandomNumberGenerator.GetBytes(...), RandomNumberGenerator.GetInt32(...), or RandomNumberGenerator.Fill(...).
Context: Random.Shared.NextDouble()
Note: [CWE-330] Use of Insufficiently Random Values.
(weak-random-system-random-csharp)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@EntityFrameworkCore.Sqlite.Concurrency/src/ThreadSafeSqliteContext.cs` around
lines 90 - 131, The retry loop in
ThreadSafeSqliteContext.WriteQueue.EnqueueAsync reuses the same DbContext after
a SQLITE_BUSY catch, so tracked Added/Modified entities can be replayed on the
next attempt. Clear the DbContext ChangeTracker before retrying inside the
busy-exception path, ideally right after catching SqliteException and before the
backoff delay or next loop iteration, so each operation((TContext)(object)this)
attempt starts from a clean state.
| // Exponential backoff with full jitter: sleep in [baseDelay, 2×baseDelay]. | ||
| var baseDelay = 100 * Math.Pow(2, attempt); | ||
| var jitter = Random.Shared.NextDouble() * baseDelay; | ||
| await Task.Delay(TimeSpan.FromMilliseconds(baseDelay + jitter), ct); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Backoff starts at 200 ms, not the documented 100 ms.
attempt is incremented before the delay is computed, so the first retry uses baseDelay = 100 * 2^1 = 200 ms. The SqliteConcurrencyOptions.MaxRetryAttempts docs state backoff starts at 100 ms. Use attempt - 1 in the exponent to match. Also note the "full jitter" comment is inaccurate — full jitter samples [0, baseDelay], whereas this yields [baseDelay, 2×baseDelay].
♻️ Align first delay with documented 100 ms
- var baseDelay = 100 * Math.Pow(2, attempt);
+ var baseDelay = 100 * Math.Pow(2, attempt - 1);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Exponential backoff with full jitter: sleep in [baseDelay, 2×baseDelay]. | |
| var baseDelay = 100 * Math.Pow(2, attempt); | |
| var jitter = Random.Shared.NextDouble() * baseDelay; | |
| await Task.Delay(TimeSpan.FromMilliseconds(baseDelay + jitter), ct); | |
| // Exponential backoff with full jitter: sleep in [baseDelay, 2×baseDelay]. | |
| var baseDelay = 100 * Math.Pow(2, attempt - 1); | |
| var jitter = Random.Shared.NextDouble() * baseDelay; | |
| await Task.Delay(TimeSpan.FromMilliseconds(baseDelay + jitter), ct); |
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 126-126: System.Random is a non-cryptographic, predictable PRNG and must not be used to generate security-sensitive values such as tokens, session IDs, passwords, salts, nonces, or API keys. An attacker who observes a few outputs can recover the seed and predict all future values. Use a cryptographically secure source instead: RandomNumberGenerator.GetBytes(...), RandomNumberGenerator.GetInt32(...), or RandomNumberGenerator.Fill(...).
Context: Random.Shared.NextDouble()
Note: [CWE-330] Use of Insufficiently Random Values.
(weak-random-system-random-csharp)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@EntityFrameworkCore.Sqlite.Concurrency/src/ThreadSafeSqliteContext.cs` around
lines 125 - 128, The retry delay calculation in ThreadSafeSqliteContext is off
by one: the first retry currently uses 200 ms because attempt is incremented
before the backoff is computed. Update the exponential backoff formula to use
attempt - 1 in the exponent so the first delay matches the
SqliteConcurrencyOptions.MaxRetryAttempts documentation at 100 ms. Also fix the
inline comment near the Task.Delay call to describe the actual jitter behavior,
since the current “full jitter” wording does not match the sampled range.
| // Or with explicit retry control | ||
| await context.ExecuteWithRetryAsync(async ctx => | ||
| { | ||
| foreach (var record in records) | ||
| ctx.Records.Add(record); | ||
| }, maxRetries: 5); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Persist the retry example.
ExecuteWithRetryAsync only stages entities here; without SaveChangesAsync() the sample never writes anything. Either commit inside the lambda or switch this snippet to the helper that auto-commits.
Proposed fix
await context.ExecuteWithRetryAsync(async ctx =>
{
foreach (var record in records)
ctx.Records.Add(record);
+ await ctx.SaveChangesAsync();
}, maxRetries: 5);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Or with explicit retry control | |
| await context.ExecuteWithRetryAsync(async ctx => | |
| { | |
| foreach (var record in records) | |
| ctx.Records.Add(record); | |
| }, maxRetries: 5); | |
| // Or with explicit retry control | |
| await context.ExecuteWithRetryAsync(async ctx => | |
| { | |
| foreach (var record in records) | |
| ctx.Records.Add(record); | |
| await ctx.SaveChangesAsync(); | |
| }, maxRetries: 5); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` around lines 128 - 133, The retry sample using
ExecuteWithRetryAsync only adds records to ctx.Records and never persists them,
so update this example to either call SaveChangesAsync inside the
ExecuteWithRetryAsync lambda or replace it with the auto-committing helper used
elsewhere in the README. Use the ExecuteWithRetryAsync snippet to locate the
example and make sure the documented flow actually writes the staged entities.
1 participant
Overview
This release delivers four categories of improvement to
EntityFrameworkCore.Sqlite.Concurrency: a core throughput architecture change, a complete documentation and SEO overhaul, a real-world benchmark suite, and a community trust package.What's Changed
Core: Channel-Based Write Queue
Replaces the per-database
SemaphoreSlim(1,1)with aChannel<IWriteRequest>-based write queue backed by a single background writer task.Why this matters: Under concurrent write load,
SemaphoreSlim.Release()wakes all N waiters simultaneously. N-1 immediately go back to sleep. This thundering-herd pattern causes CPU spikes, unfair scheduling, and elevated context-switch counts at high concurrency. AChannel<T>parks callers in FIFO order and drains them one at a time, eliminating the wakeup storm entirely.SqliteWriteQueueclass owns theChannel<IWriteRequest>and the background writer loopWriteQueueCapacityoption added toSqliteConcurrencyOptions:null= unbounded (default), integer = bounded channel withBoundedChannelFullMode.Waitfor producer back-pressureIsWriteLockHeldreentrancy guard moved fromSqliteConnectionEnhancerintoSqliteWriteQueueasAsyncLocal<bool> IsWriteLoopExecutingBug fixes included:
ThreadSafeSqliteContext.ExecuteWriteAsyncwas constructingnew SqliteConcurrencyOptions(), ignoring registered options. Fixed viaTryGetInterceptorlookup.BulkInsertSafeAsyncusedSkip/Takein a loop (O(n^2) LINQ-to-objects). Replaced withChunk(1000).BulkInsertSafeAsyncdid not callChangeTracker.Clear()between batches, causing memory to grow linearly with entity count. Now cleared after each batchSaveChangesAsync.netstandard 2.0 TFM
The connection and queue layer (
SqliteConnectionEnhancer,SqliteWriteQueue,SqliteConcurrencyOptions) now targetsnetstandard2.0in addition tonet10.0. EF Core-dependent APIs remainnet10.0-only (EF Core 10 requires it).BenchmarkDotNet Suite
New
EFCore.Sqlite.Concurrency.Benchmarksproject with three benchmark classes:BulkInsertBenchmarksSaveChangesAsync-per-entity vsBulkInsertOptimizedAsyncat 1,000 and 10,000 entitiesConcurrentWriteBenchmarksTask.WhenAllwith 10 and 50 concurrent writers viaSaveChangesSerializedAsyncReadBenchmarksCountAsyncreaders: default journal mode vs WAL modeMeasured results (BenchmarkDotNet v0.14.0, .NET 10.0.2, Windows 11, Intel i7-13700K):
SQLITE_BUSYRun:
dotnet run -c Release --project EFCore.Sqlite.Concurrency.Benchmarks -- --job shortDocumentation and SEO
Complete problem-first rewrite of all user-facing surfaces so the package is discoverable when developers search for
SQLITE_BUSY C#,database is locked EF Core,SQLITE_BUSY_SNAPSHOT, and related terms:Task.WhenAll, background service, and request-scoped patterns with working codeUseSqlite, common pitfalls checklistCI and Trust Signals
.github/workflows/ci.ymlwithdorny/test-reporter@v1for TRX result publishingSQLitePCLRaw.lib.e_sqlite32.1.11 is both the latest published version and carries advisory GHSA-2m69-gcr7-jv3q; no patched release exists on NuGet yet. Advisory-scopedNuGetAuditSuppressadded with a comment to remove once upstream ships a fix.Test Plan
dotnet build -c Release- 0 errors, NU1903 warning resolveddotnet test EFCore.Sqlite.Concurrency.Test/ --framework net10.0- all 4 stress tests pass (50 concurrent writers,ThreadSafeSqliteContext,SaveChangesSerializedAsync, mixed read/write)dotnet pack -c Release- package builds successfullyBreaking Changes
None. All existing public API signatures are preserved.
WriteQueueCapacityis additive. Thenetstandard2.0TFM is additive.