Skip to content

One SqliteConnection is shared across SearchService and every OutputIndexer without synchronisation #102

Description

@AThraen

Observed in a live crash.log during session restore:

[16.22.23] TASK: System.AggregateException: A Task's exception(s) were not observed ...
 ---> System.ArgumentOutOfRangeException: Index was out of range. (Parameter 'index')
   at Microsoft.Data.Sqlite.SqliteCommand.Dispose(Boolean disposing)
   at System.Data.Common.DbCommand.DisposeAsync()
   at CodeShellManager.Services.SearchService.RecordSessionStartAsync(String command)

Root cause

MainWindow opens one SqliteConnection and hands the same instance to:

  • SearchService — 13 public async methods, 15 _db.CreateCommand() sites
  • every OutputIndexer — one per session, each draining its own channel on a worker

SqliteConnection is not thread-safe. It keeps an internal list of live commands; concurrent create/dispose from different threads corrupts it, which is exactly the ArgumentOutOfRangeException inside SqliteCommand.Dispose above.

The restore loop makes this easy to hit: every launching session fires _ = _searchService.RecordSessionStartAsync(...) fire-and-forget while every already-started session's indexer is writing output. With 41 sessions that's a lot of concurrent traffic on one connection.

This may also explain the shutdown NRE we currently swallow

MainWindow.OnClosing has:

// OutputIndexer.Dispose now drains its worker first, but SqliteConnection.Close
// has been observed to throw NRE internally on shutdown — swallow + log so it
// doesn't escape as an unhandled exception during application exit.
try { _db?.Close(); }
catch (Exception ex) { Log($"OnClosing _db.Close threw: {ex}"); }

Those entries are all over crash.log. An internally-corrupted connection throwing on Close is consistent with the same root cause. Worth re-checking whether that swallow is still needed once access is synchronised — if it is, that's a separate bug rather than a mystery.

Why it's easy to miss

The failure is an unobserved task exception. RecordSessionStartAsync is invoked as _ = ..., so nothing awaits it and nothing surfaces the fault until the finalizer rethrows it later, detached from the code that caused it. Everything appears to work; rows just go missing.

Fix

Serialise access to the shared connection. Options considered:

  1. One async gate around all DB work — simplest and complete. SQLite writes here are small and the indexer already batches through a channel, so serialising is unlikely to be felt.
  2. Connection-per-OutputIndexer plus WAL — better theoretical concurrency, but a bigger change and still leaves SearchService's own methods racing each other.

Going with (1).

Also worth fixing while here

_ = _searchService.RecordSessionStartAsync(...) discards the task. Even once synchronised, a genuine failure stays invisible. It should at least log on fault.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions