NpgsqlDataReader is being disposed when using the latest version of Npgsql with Insight.

[ctomichael]

Describe the bug

When I use new NpgsqlConnection("connectionString").As<IRepository> to set up a repository, and then attempt to run a query using the latest version of Npgsql and Insight.Database, the request to the database throws an ObjectDisposedException:

ObjectDisposedException: Cannot access a disposed object. Object name: 'NpgsqlDataReader'.
Npgsql.NpgsqlDataReader.NextResult(bool async, bool isConsuming, CancellationToken cancellationToken)
Npgsql.NpgsqlDataReader.NextResult()
Insight.Database.DBConnectionExtensions.ExecuteAsyncAndAutoClose<T>(IDbConnection connection, object parameters, Func<IDbConnection, IDbCommand> getCommand, bool callGetReader, Func<IDbCommand, IDataReader, Task<T>> translate, CommandBehavior commandBehavior, CancellationToken cancellationToken, object outputParameters)

Steps to reproduce

public interface IRepository 
{
        [Sql(@"
            SELECT * FROM "Activities"
        ")]
        Task<IList<Activity>> GetActivities();
}

...

var repository = new NpgsqlConnection("connectionString").As<IRepository>();
var result = await repository.GetActivities();

Expected behavior

The query would be run and returned successfully

• Dotnet version: netcore 3.1
• Database: Postgres (Npgsql 5.0.3)
• Library version: 6.3.4

[jonwagner]

There haven’t been any changes to that driver or the record reading functionality in a while. There’s a possibility that there is a breaking change in npgsql.

Can you try it with older versions of npgsql and insight to try to narrow it down?

On Feb 18, 2021, at 7:24 PM, Michael McClenaghan notifications@github.com wrote:



Describe the bug

When I use new NpgsqlConnection("connectionString").As to set up a repository, and then attempt to run a query using the latest version of Npgsql and Insight.Database, the request to the database throws an ObjectDisposedException:

ObjectDisposedException: Cannot access a disposed object. Object name: 'NpgsqlDataReader'. Npgsql.NpgsqlDataReader.NextResult(bool async, bool isConsuming, CancellationToken cancellationToken) Npgsql.NpgsqlDataReader.NextResult() Insight.Database.DBConnectionExtensions.ExecuteAsyncAndAutoClose(IDbConnection connection, object parameters, Func<IDbConnection, IDbCommand> getCommand, bool callGetReader, Func<IDbCommand, IDataReader, Task> translate, CommandBehavior commandBehavior, CancellationToken cancellationToken, object outputParameters)

Steps to reproduce

public interface IRepository { [Sql(@" SELECT * FROM "Activities" ")] Task<IList> GetActivities(); }

...

var repository = new NpgsqlConnection("connectionString").As(); var result = await repository.GetActivities();

Expected behavior

The query would be run and returned successfully

• Dotnet version: netcore 3.1
• Database: Postgres (Npgsql 5.0.3)
• Library version: 6.3.4

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/jonwagner/Insight.Database/issues/452, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAMTO5FIAJMV777GCENWZ4TS7WVTDANCNFSM4X3M76BA.

[ctomichael]

Hi Jon,

I've done some testing on a bare project, and I can confirm that there is a breaking change between Npgsql 4.1.8 and 5.0.0. It works fine with any version prior to 5.0.0, but after that I receive the error posted above every time. I've had a look at the breaking changes section of 5.0, and I can't see anything glaringly obvious as I'm not super familiar with how Insight manages the connection under the hood.

https://www.npgsql.org/doc/release-notes/5.0.html#breaking-changes

[jonwagner]

My guess is that when it reads the final recordset on a reader, it disposes the reader, making the call to NextRecordset fail.

The original npgsql was a clone of other sql clients and had common behavior. The 4.x/5.x rewrites of npgsql have caused a number of these minor issues.

One of us will dig up some time to patch the provider.

On Feb 22, 2021, at 7:25 PM, Michael McClenaghan notifications@github.com wrote:



Hi Jon,

I've done some testing on a bare project, and I can confirm that there is a breaking change between Npgsql 4.1.8 and 5.0.0. It works fine with any version prior to 5.0.0, but after that I receive the error posted above every time. I've had a look at the breaking changes section of 5.0, and I can't see anything glaringly obvious as I'm not super familiar with how Insight manages the connection under the hood.

https://www.npgsql.org/doc/release-notes/5.0.html#breaking-changes

— You are receiving this because you commented. Reply to this email directly, view it on GitHubhttps://github.com/jonwagner/Insight.Database/issues/452#issuecomment-783776774, or unsubscribehttps://github.com/notifications/unsubscribe-auth/AAMTO5A7NSAVIFK6QXABBLTTALYX5ANCNFSM4X3M76BA.

[ctomichael]

Thanks Jon. Appreciate your work on this!

[ctomichael]

Hi Jon. Just wondering if there is an update on this? Is there anything I can do to assist here? Happy to take a look if you point me in the right direction.

[jonwagner]

The good news: easy to reproduce with npgsql5.0. The bad news: it's in the way they are handling returning recordsets. They made major breaking changes going from 3.0 to 4.0, which we were able to work around. Let's see if it's easy to support in 5.0...

[jonwagner]

Updated good news: it looks like the issue is that Insight has an unnecessary call to Dispose in the AsyncReader class, and the latest npgsql just got more strict about touching disposed readers. Updated bad news: it's kinda scary to rip that Dispose out so I'll want a few people to run some tests.

[jonwagner]

@lawrencek76 @Jaxelr do you have time to run some real tests with the latest in the main branch? Removing the dispose call shouldn't be an issue, but it's in the middle of AsyncReader so I'd want to double check.

[lawrencek76]

@jonwagner can you publish a prerelease to nuget I can put it through some testing

[Jaxelr]

yes, that would be helpful since i could test by deploying on some canary environments more easily without disrupting CIs

[jonwagner]

v6.3.5-preview is now available in nuget with this fix in it.

[codewithdebasis]

hi @jonwagner I just want to inform you one good news that previously I got the same error in my project in asp.net core 5.0 with v6.3.4. But after update the v6.3.5 preview it is now solved and it worked properly.

[Jaxelr]

So far we've been running with v6.3.5 and no regression errors have been found, currently using the default, postgres and mssqlclient providers

[jonwagner]

Perfect. v6.3.5 is now available.

Thanks @Jaxelr @debasis-saha