SqliteDataReader.Read throws "Invalid attempt to call Read when reader is closed."

[ryepup]

I'm using sqlite in-memory databases as a lightweight way to test code that will normally talk to Sql Server.

I'm writing some code that expects to deal with large result sets and stream data from the database without holding the entire dataset in memory. That code is written in terms of IDbConnection.

I'm running into unexpected errors where my test works with a SqlConnection, but fails with a SqliteConnection.

The code I'm testing has a few layers, but the error can be reproduced with only Microsoft.Data.Sqlite.

Reproduction case

Install Microsoft.Data.Sqlite, run this mstest:

using System.Data;
using System.Threading.Tasks;
using Microsoft.Data.Sqlite;
using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace SqliteRepro
{
    [TestClass]
    public class SqliteReproTests
    {
        [TestMethod]
        public void SqliteCommand_CommandDisposes_LeavesReaderOpen()
        {
            using IDbConnection conn = new SqliteConnection("Data Source=:memory:");
            conn.Open();
            IDataReader reader = null;
            using (IDbCommand command = conn.CreateCommand())
            {
                command.CommandText = "SELECT 1";
                reader = command.ExecuteReader();
                Assert.IsFalse(reader.IsClosed);
            }
            Assert.IsFalse(reader.IsClosed, "disposing the command closes the reader?");
        }
    }
}

Expected behavior

The test passes, and disposing the IDbCommand has no effect on the IDataReader.

Actual behavior

Test fails:

$ dotnet test
Test run for /PATH/bin/Debug/netcoreapp3.1/Tests.dll(.NETCoreApp,Version=v3.1)
Microsoft (R) Test Execution Command Line Tool Version 16.7.0
Copyright (c) Microsoft Corporation.  All rights reserved.

Starting test execution, please wait...

A total of 1 test files matched the specified pattern.
  X SqliteCommand_CommandDisposes_LeavesReaderOpen [76ms]
  Error Message:
   Assert.IsFalse failed. disposing the command closes the reader?
  Stack Trace:
     at SqliteRepro.SqliteReproTests.SqliteCommand_CommandDisposes_LeavesReaderOpen() in /.../SqliteRepro.cs:line 23
   at Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices.ThreadOperations.ExecuteWithAbortSafety(Action action)

Any attempts to call reader.Read() throw:

System.InvalidOperationException: Invalid attempt to call Read when reader is closed.
  Stack Trace:
      at Microsoft.Data.Sqlite.SqliteDataReader.Read()

Analysis

If you replace my new SqliteConnection("Data Source=:memory:") with a new SqlConnection pointing towards a read SQL server, the test passes.

I think the difference in behavior comes from SqliteCommand vs SqlCommand.

SqliteCommand holds a reference to any data readers it opens:

efcore/src/Microsoft.Data.Sqlite.Core/SqliteCommand.cs

Line 315 in ea79edc

return DataReader = dataReader;

When SqliteCommand.Dispose is called, any related reader is also disposed:

efcore/src/Microsoft.Data.Sqlite.Core/SqliteCommand.cs

Line 522 in ea79edc

DataReader.Dispose();

SqlCommand does not keep references to its readers nor attempt to clean them up; those are the caller's responsibility.

I think this behavior makes SqliteCommand go beyond the contract of IDbCommand.ExecuteReader.

Versions

Microsoft.Data.Sqlite version: 5.0.2 and 3.1.11
Target framework: netcoreapp3.1
Operating system: ubuntu20.04 under WSL2 under win10

[roji]

@ryepup I don't think there's any explicit contract somewhere saying that a reader must outlive its command - even if that's the SqlClient behavior. In fact, since a reader is obtained from a command (i.e. by executing it), there's some sense in the reader not working beyond the scope of the command's lifecycle; just like the reader is invalidated once its connection is closed/disposed.

On the flip side, compatbility with the SqlClient behavior is definitely a plus (Npgsql also allows leaves the reader working after command disposal).

[ryepup]

@roji true, but there's also no contract saying that the command's lifecycle and the reader's lifecycle are related.

From IDbCommand.ExecuteReader docs:

Executes the CommandText against the Connection, and returns an DbDataReader.

I interpret that to mean the command has done it's job, and the rest is between the Connection and the DbDataReader.

C# doesn't give us many tools to express this kind of issue, best I can think of is either:

• decouple the lifetimes
• add more docs to mention this behavior, at which point it will still be a subtle bug that will surprise devs

[roji]

@roji true, but there's also no contract saying that the command's lifecycle and the reader's lifecycle are related.

Yep. ADO.NET is a notoriously ambiguous spec. I do agree with you that given that SqlClient and Npgsql allow it, it's probably preferable for Sqlite to also allow it - unless there's some specific implementation issue there.

[ajcvickers]

Note from triage: the behavior of SQLite here will change based on the implementation of #14044. This should allow disposing of the command before disposing of the data reader. (Note that some providers don't require that the DbCommand be disposed. However, in general ADO.NET requires that DbCommand be disposed. We are not changing this, even though SQLite may join those providers that do not need it.)

[mkmitchell79]

Bumped into this issue trying to migrate to Microsoft.Data.Sqlite. Our application allows the use of either MySQL, SQL Server, Oracle, or Mono.Data.Sqlite for many years and none of these providers have a problem with IDataReader outliving the command.

Any plans to get this fixed?

[roji]

This issue is in the Backlog milestone. This means that it is not planned for the next release (EF Core 8.0). We will re-assess the backlog following the this release and consider this item at that time. However, keep in mind that there are many other high priority features with which it will be competing for resources. Make sure to vote (👍) for this issue if it is important to you.

[mkmitchell79]

@roji - Appreciate the fast response. FYI, our use case is a bit different from the original reporter. We're attempting to use the library directly, outside of ef, on Android devices as part of migrating away from Xamarin to NET8.