Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NpgsqlDatabaseCreator.Exist doesn't like DbConnectionInterceptor to open the connection #3455

Open
foriequal0 opened this issue Feb 6, 2025 · 2 comments

Comments

@foriequal0
Copy link

We somehow had a DbConnectionInterceptor that opens a connection on ConnectionOpening. I thought it could be done

In EF Core, ConnectionOpened is invoked regardless of IsSuppressed value, so the interceptor should've opened the connection when IsSuppressed is true.
https://github.com/dotnet/efcore/blob/6cacf97e1093965928d27e99a7b3f90d25c0b01b/src/EFCore.Relational/Storage/RelationalConnection.cs#L758-L767

But after this change (#3349), we had errors (reproduced below) and connection leaks (couldn't reproduce in the test).

Here's the repro:

public sealed class TestContext : DbContext
{
    public TestContext(DbContextOptions<TestContext> options)
        : base(options)
    {
    }
}

public sealed class ConnectionOpeningInterceptor : DbConnectionInterceptor
{
    public override async ValueTask<InterceptionResult> ConnectionOpeningAsync(
        DbConnection connection, ConnectionEventData eventData, InterceptionResult result,
        CancellationToken cancellationToken = default)
    {
        await connection.OpenAsync(cancellationToken);
        return InterceptionResult.Suppress();
    }
}

var services = new ServiceCollection();
services.AddDbContext<TestContext>(options =>
{
    options.UseNpgsql("connection string");
    options.AddInterceptors(new ConnectionOpeningInterceptor());
});

await using var serviceProvider = services.BuildServiceProvider();
var context = serviceProvider.GetRequiredService<TestContext>();
var relationalDatabaseCreator = context.GetService<IRelationalDatabaseCreator>();

await relationalDatabaseCreator.ExistsAsync(); // okay, but doesn't close connection
await relationalDatabaseCreator.ExistsAsync(); // throws exception on the second invocation

I think the change slightly broke the semantics of the interceptor.
Before the change, The invocation of ConnectionOpening was always followed by the matching invocations of ConnectionOpened and ConnectionClosing or ConnectionDisposing if there were no errors.
But after the change, there is always one more invocation of ConnectionOpening

We removed the interceptor, so it doesn't necessarily need to be fixed for me, but I leave this issue as a future note.

@roji
Copy link
Member

roji commented Feb 6, 2025

@foriequal0 just so I'm clear on things, are you saying that EF should be closing the connection even though ConnectionOpening was suppressed?

@foriequal0
Copy link
Author

foriequal0 commented Feb 6, 2025

I think Npgsql.EFCore should.

For other normal queries (e.g. await context.Database.SqlQueryRaw<int>($"SELECT 1").ToListAsync();), we get

  • a Connection is not open error if we suppress but don't open the connection.
  • a Connection already open error if we open the connection, but don't suppress.

So the interceptor must open the connection if and only if it suppresses.
If it doesn't suppress, EF opens the connection internally.
The connection is opened anyway, so EF closes it appropriately eventually.

However NpgsqlDatabaseCreator.Exist calls the ConnectionOpening (which opens the connection when suppressed), but it doesn't close the connection.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants