MySqlConnector icon indicating copy to clipboard operation
MySqlConnector copied to clipboard

NullReferenceException in MySqlConnector.MySqlDataReader.ActivateResultSet

Open feiazifeiazi opened this issue 1 year ago • 12 comments
trafficstars

Software versions MySqlConnector version: 2.3.5 Server type (MySQL, MariaDB, Aurora, etc.) and version: 8.0.25 .NET version: net7.0 Dapper version: 2.1.28

Describe the bug NullReferenceException

Exception

[Error Message]=Object reference not set to an instance of an object.
[Exception Type]=System.NullReferenceException
[Source]=MySqlConnector
[TargetSite]=Void ActivateResultSet(System.Threading.CancellationToken)
[Stack Trace]=   at MySqlConnector.MySqlDataReader.ActivateResultSet(CancellationToken cancellationToken) in /_/src/MySqlConnector/MySqlDataReader.cs:line 133
   at MySqlConnector.MySqlDataReader.InitAsync(CommandListPosition commandListPosition, ICommandPayloadCreator payloadCreator, IDictionary`2 cachedProcedures, IMySqlCommand command, CommandBehavior behavior, Activity activity, IOBehavior ioBehavior, CancellationToken cancellationToken) in /_/src/MySqlConnector/MySqlDataReader.cs:line 505
   at MySqlConnector.Core.CommandExecutor.ExecuteReaderAsync(CommandListPosition commandListPosition, ICommandPayloadCreator payloadCreator, CommandBehavior behavior, Activity activity, IOBehavior ioBehavior, CancellationToken cancellationToken) in /_/src/MySqlConnector/Core/CommandExecutor.cs:line 78
   at MySqlConnector.MySqlCommand.ExecuteNonQueryAsync(IOBehavior ioBehavior, CancellationToken cancellationToken) in /_/src/MySqlConnector/MySqlCommand.cs:line 317
   at MySqlConnector.MySqlCommand.ExecuteNonQuery() in /_/src/MySqlConnector/MySqlCommand.cs:line 108
   at Dapper.SqlMapper.ExecuteCommand(IDbConnection cnn, CommandDefinition& command, Action`2 paramReader) in /_/Dapper/SqlMapper.cs:line 2975
   at Dapper.SqlMapper.ExecuteImpl(IDbConnection cnn, CommandDefinition& command) in /_/Dapper/SqlMapper.cs:line 656
   at Wwtfly.DBMonitor.DAL_Dapper.DBLinkConfigDAL.UpdateExecCount()

Code sample

-

Expected behavior

Additional context This method is an update SQL . Its execution frequency is approximately 50 times per second.

The error randomly occurs 20 times per day.

Link to the exception line

feiazifeiazi avatar Mar 06 '24 02:03 feiazifeiazi

Can you provide a self-contained example that reproduces the problem?

A NullReferenceException in MySqlConnector is most typically indicative of broken internal invariants, usually caused by multithreaded misuse. See https://mysqlconnector.net/troubleshooting/connection-reuse/ and note that connections must not be accessed by multiple threads at the same time.

bgrainger avatar Mar 06 '24 03:03 bgrainger

Related: https://github.com/mysql-net/MySqlConnector/issues/1427#issuecomment-1872985495

bgrainger avatar Mar 06 '24 03:03 bgrainger

Can you provide a self-contained example that reproduces the problem?

A NullReferenceException in MySqlConnector is most typically indicative of broken internal invariants, usually caused by multithreaded misuse. See https://mysqlconnector.net/troubleshooting/connection-reuse/ and note that connections must not be accessed by multiple threads at the same time.

thank you very much. I will study the code. (This is old code, converted from Oracle MySQL driver.)

feiazifeiazi avatar Mar 06 '24 08:03 feiazifeiazi

I'm seeing the same thing.

Nothing database-related changed other than updating the versions of MySqlConnector and Dapper earlier this week.

I looked for threading issues. The places where database connections are created and used are pretty minimal (abstracted into some common code). Every connection create has a using statement and there aren't any places where the connection is created and then returned to the caller.

I'm going to try rolling back to an older version to see what happens.

More Context:

  • Upgraded MySqlConnector from 2.1.11 to 2.3.5
  • Upgrade Dapper from 2.0.123 to 2.1.35

Sometimes there is a null reference during Dispose:

   at MySqlConnector.MySqlDataReader.DisposeAsync(IOBehavior ioBehavior, CancellationToken cancellationToken) in /_/src/MySqlConnector/MySqlDataReader.cs:line 612
   at System.Data.Common.DbDataReader.Dispose(Boolean disposing)
   at MySqlConnector.MySqlDataReader.Dispose(Boolean disposing) in /_/src/MySqlConnector/MySqlDataReader.cs:line 443
   at Dapper.SqlMapper.QueryAsync[T](IDbConnection cnn, Type effectiveType, CommandDefinition command) in /_/Dapper/SqlMapper.Async.cs:line 472

Sometimes there is an null reference during ActivateResultSet:

   at MySqlConnector.MySqlDataReader.ActivateResultSet(CancellationToken cancellationToken) in /_/src/MySqlConnector/MySqlDataReader.cs:line 117
   at MySqlConnector.MySqlDataReader.InitAsync(CommandListPosition commandListPosition, ICommandPayloadCreator payloadCreator, IDictionary`2 cachedProcedures, IMySqlCommand command, CommandBehavior behavior, Activity activity, IOBehavior ioBehavior, CancellationToken cancellationToken) in /_/src/MySqlConnector/MySqlDataReader.cs:line 483
   at MySqlConnector.Core.CommandExecutor.ExecuteReaderAsync(CommandListPosition commandListPosition, ICommandPayloadCreator payloadCreator, CommandBehavior behavior, Activity activity, IOBehavior ioBehavior, CancellationToken cancellationToken) in /_/src/MySqlConnector/Core/CommandExecutor.cs:line 56
   at MySqlConnector.MySqlCommand.ExecuteReaderAsync(CommandBehavior behavior, IOBehavior ioBehavior, CancellationToken cancellationToken) in /_/src/MySqlConnector/MySqlCommand.cs:line 357
   at MySqlConnector.MySqlCommand.ExecuteDbDataReaderAsync(CommandBehavior behavior, CancellationToken cancellationToken) in /_/src/MySqlConnector/MySqlCommand.cs:line 350

There is a packet out-of-order error, too:

One or more errors occurred. (Packet received out-of-order. Expected 1; got 2.)

I don't have a usable stack trace for the last one, since it is wrapped up in an AggregateException

jayrowe avatar Mar 15 '24 15:03 jayrowe

Sometimes there is a null reference during Dispose:

   at MySqlConnector.MySqlDataReader.DisposeAsync(IOBehavior ioBehavior, CancellationToken cancellationToken) in /_/src/MySqlConnector/MySqlDataReader.cs:line 612
   at System.Data.Common.DbDataReader.Dispose(Boolean disposing)
   at MySqlConnector.MySqlDataReader.Dispose(Boolean disposing) in /_/src/MySqlConnector/MySqlDataReader.cs:line 443
   at Dapper.SqlMapper.QueryAsync[T](IDbConnection cnn, Type effectiveType, CommandDefinition command) in /_/Dapper/SqlMapper.Async.cs:line 472

This call stack doesn't "look right". I wonder if there's an earlier exception that's being thrown, but this NullReferenceException is coming from a second exception being thrown while finally blocks are being run.

bgrainger avatar Mar 18 '24 15:03 bgrainger

@bgrainger

I agree, but if you look at that stack trace, you can see that it's in Dapper. Dapper is opening and closing the connections and disposing the reader for me. I'm really not managing connections other than creating them and calling Dispose or DisposeAsync. I'm dealing with the readers even less frequently.

I rolled back to 2.1.11 yesterday midday, and it hasn't happened since. It was happening quite consistently since I upgraded. The chart of error counts related to this is pretty damning so far. You can see when I upgraded and when I downgraded; usage has been consistent throughout the time period.

image

jayrowe avatar Mar 19 '24 13:03 jayrowe

I work on systems with MySqlConnector 2.3.5 deployed to production and opening about 1,000,000 database connections per hour. We have not seen widespread NullReferenceExceptions like you're reporting.

Are you able to provide a small repro that demonstrates the problem?

Or can you provide real (not abbreviated or sanitised) examples of what your calling code (with Dapper) looks like?

bgrainger avatar Mar 19 '24 16:03 bgrainger

I tried inducing the failure before I rolled back but was unable to do it on demand.

It's pretty bog standard. Things are abstracted for instrumentation purposes so the failing methods generally are one of these:

    protected async Task<List<TResult>> QueryAsync<TResult>(string sql, object? parameters = null, CommandFlags flags = CommandFlags.Buffered, CancellationToken cancellationToken = default)
    {
        flags |= CommandFlags.Buffered;

        using var connection = _connectionFactory.Create();

        return (List<TResult>)await connection.QueryAsync<TResult>(
            new CommandDefinition(
                sql,
                parameters,
                flags: flags,
                cancellationToken: cancellationToken));
    }

    protected async Task ExecuteAsync(string sql, object? parameters = null, CancellationToken cancellationToken = default)
    {
        using var connection = _connectionFactory.Create();

        await connection.ExecuteAsync(
            new CommandDefinition(sql, parameters, cancellationToken: cancellationToken));
    }

    protected async Task<TResult> ExecuteScalarAsync<TResult>(string sql, object? parameters = null, CancellationToken cancellationToken = default)
    {
        using var connection = _connectionFactory.Create();

        return await connection.ExecuteScalarAsync<TResult>(
            new CommandDefinition(sql, parameters, cancellationToken: cancellationToken));
    }

_connectionFactory.Create() just creates the new instance with the proper connection string as there are separate databases and users with different permissions.

I looked through the cases not using one of those methods and most follow the exact same pattern - create connection with a using statement, call Dapper. The ones that don't follow the pattern are where there is a low number of iterations calling a stored procedure to do something complicated, so the code calls OpenAsync on the connection once and leaves it open.

It's happening across different connections to different databases with different connection strings, and rolling back made the problem disappear for the last 24+ hours after having been consistent for five or so days.

I'm on .NET 8 and I'm connecting to an AWS Aurora MySQL cluster if either of those is a complicating factor.

jayrowe avatar Mar 19 '24 19:03 jayrowe

Thanks for the update. That code looks pretty "bog standard", as you said. CommandFlags.Buffered is a little unusual (I don't think most Dapper users use it), but shouldn't cause any problems.

Does _connectionFactory.Create() also open the connection, or do you (almost always) let Dapper open and close the connection?

bgrainger avatar Mar 19 '24 20:03 bgrainger

It just creates the connection and returns it; it does not open it. With very few exceptions I let Dapper open and close the connection.

I'm swamped right now but I'll try to circle back when I get a chance. I can probably deploy a version to my staging environment with an instrumented build to try to catch it in action. It happens in that environment but the frequency of data updates is much lower so it isn't as consistent as it was in my production environment.

jayrowe avatar Mar 20 '24 12:03 jayrowe

I see you're passing in cancellationToken: cancellationToken. Is it possible that these exceptions are correlated with cancelled queries? Do you know how frequently cancellation is happening?

bgrainger avatar Mar 20 '24 17:03 bgrainger

I would expect it to be rare; the case would be a cancelled http request in a user-facing path. Most of our content is served up from OpenSearch, not from the database.

The paths throwing exceptions are in background message processing for data updates and those don't pass in any CancellationToken.

jayrowe avatar Mar 20 '24 17:03 jayrowe

+1 we are seeing the exact same issue, with Dapper 2.0.90 and MySqlConnector 2.3.7, on .NET 6. Pretty standard usage, no crazy black magic with threads as far as I can tell.

lazorfuzz avatar Apr 10 '24 23:04 lazorfuzz

Is anyone who is experiencing this problem able to provide a self-contained example that reproduces the exception?

bgrainger avatar Apr 13 '24 19:04 bgrainger

Upgraded MySqlConnector from 2.1.11 to 2.3.5

One major change in 2.3.5 was https://github.com/mysql-net/MySqlConnector/issues/1277 which recycles MySqlDataReader instances. Having two threads use the same instance at once would be a significant bug (and misuse of the library).

at MySqlConnector.MySqlDataReader.ActivateResultSet(CancellationToken cancellationToken) in /_/src/MySqlConnector/MySqlDataReader.cs:line 133

The only thing that could be null on https://github.com/mysql-net/MySqlConnector/blob/2.3.5/src/MySqlConnector/MySqlDataReader.cs#L133 is m_resultSet.ReadResultSetHeaderException but we're inside an if block that checks if it's not null.

at MySqlConnector.MySqlDataReader.ActivateResultSet(CancellationToken cancellationToken) in /_/src/MySqlConnector/MySqlDataReader.cs:line 117

This line https://github.com/mysql-net/MySqlConnector/blob/2.3.5/src/MySqlConnector/MySqlDataReader.cs#L117 also can't throw, but the lines before and after also read m_resultSet.ReadResultSetHeaderException.

This value is set to null in ResultSet.Reset; if that were somehow being called on another thread, we could see this exception.

bgrainger avatar Apr 14 '24 21:04 bgrainger

Is anyone who is experiencing this problem able to provide a self-contained example that reproduces the exception?

Another thing that might help is providing trace-level logging leading up to the exception.

bgrainger avatar Apr 14 '24 21:04 bgrainger

Hey. We have the same issue after upgrading MySqlConnector to 2.3.5

Context: Upgraded MySqlConnector to 2.3.5 Upgrade Dapper to 2.1.35

The downgrade MySqlConnector to 2.3.1 does not help. BTW, here https://github.com/mysql-net/MySqlConnector/issues/1427#issuecomment-1872985495
on MySqlConnector v2.3.1 it leads to the same behavior.

Here is a Stack trace from MySqlConnector v2.3.1:

System.NullReferenceException: Object reference not set to an instance of an object.
at MySqlConnector.MySqlDataReader.ActivateResultSet(CancellationToken cancellationToken) in /_/src/MySqlConnector/MySqlDataReader.cs:line 117
at MySqlConnector.MySqlDataReader.InitAsync(CommandListPosition commandListPosition, ICommandPayloadCreator payloadCreator, IDictionary`2 cachedProcedures, IMySqlCommand command, CommandBehavior behavior, Activity activity, IOBehavior ioBehavior, CancellationToken cancellationToken) in /_/src/MySqlConnector/MySqlDataReader.cs:line 483
at MySqlConnector.Core.CommandExecutor.ExecuteReaderAsync(CommandListPosition commandListPosition, ICommandPayloadCreator payloadCreator, CommandBehavior behavior, Activity activity, IOBehavior ioBehavior, CancellationToken cancellationToken) in /_/src/MySqlConnector/Core/CommandExecutor.cs:line 56
at MySqlConnector.MySqlCommand.ExecuteReaderAsync(CommandBehavior behavior, IOBehavior ioBehavior, CancellationToken cancellationToken) in /_/src/MySqlConnector/MySqlCommand.cs:line 357
at MySqlConnector.MySqlCommand.ExecuteDbDataReaderAsync(CommandBehavior behavior, CancellationToken cancellationToken) in /_/src/MySqlConnector/MySqlCommand.cs:line 350
at Datadog.Trace.ClrProfiler.CallTarget.Handlers.Continuations.TaskContinuationGenerator`4.SyncCallbackHandler.ContinuationAction(Task`1 previousTask, TTarget target, CallTargetState state)
at Dapper.SqlMapper.QueryRowAsync[T](IDbConnection cnn, Row row, Type effectiveType, CommandDefinition command) in /_/Dapper/SqlMapper.Async.cs:line 489 

Connections are not disposed explicitly, "await using" used for that.

await using var connection = new MySqlConnection(_mySqlConnectionString);
var entities = await connection.QueryAsync<DbEntity>(cmdDef);
return entities; 

Vofkin-San avatar Apr 19 '24 13:04 Vofkin-San

I created a test program to issue a large number of simultaneous queries to MySQL Server with Dapper (using QueryAsync<T>), using combinations of the following variations:

  • mysql:8.3 Docker container and Azure Database for MySQL
  • CommandFlags.None and CommandFlags.Buffered
  • no CancellationToken and CancellationTokenSource.CancelAfter
  • net8.0 and net48
  • 10 rows of 2 columns and 1,000 rows of 6 columns
  • MaximumPoolSize of 100 and 250

After several dozen runs, I never saw a NullReferenceException thrown.

At this point I think I need someone who's experiencing the problem to provide either a repro or detailed logs.

bgrainger avatar Apr 20 '24 23:04 bgrainger

I wonder if there's an earlier exception that's being thrown, but this NullReferenceException is coming from a second exception being thrown while finally blocks are being run.

I think this is what's happening. I was able to reproduce the following two call stacks by deliberately injecting errors into the MySQL protocol being returned to MySqlConnector (and ignoring them with catch (MySqlProtocolException):

Unhandled exception. System.NullReferenceException: Object reference not set to an instance of an object.
   at MySqlConnector.MySqlDataReader.ActivateResultSet(CancellationToken cancellationToken) in /_/src/MySqlConnector/MySqlDataReader.cs:line 118
   at MySqlConnector.MySqlDataReader.InitAsync(CommandListPosition commandListPosition, ICommandPayloadCreator payloadCreator, IDictionary`2 cachedProcedures, IMySqlCommand command, CommandBehavior behavior, Activity activity, IOBehavior ioBehavior, CancellationToken cancellationToken) in /_/src/MySqlConnector/MySqlDataReader.cs:line 483
   at MySqlConnector.Core.CommandExecutor.ExecuteReaderAsync(CommandListPosition commandListPosition, ICommandPayloadCreator payloadCreator, CommandBehavior behavior, Activity activity, IOBehavior ioBehavior, CancellationToken cancellationToken) in /_/src/MySqlConnector/Core/CommandExecutor.cs:line 56
   at MySqlConnector.MySqlCommand.ExecuteReaderAsync(CommandBehavior behavior, IOBehavior ioBehavior, CancellationToken cancellationToken) in /_/src/MySqlConnector/MySqlCommand.cs:line 357
   at MySqlConnector.MySqlCommand.ExecuteDbDataReaderAsync(CommandBehavior behavior, CancellationToken cancellationToken) in /_/src/MySqlConnector/MySqlCommand.cs:line 350
   at Dapper.SqlMapper.QueryAsync[T](IDbConnection cnn, Type effectiveType, CommandDefinition command) in /_/Dapper/SqlMapper.Async.cs:line 434
Unhandled exception. System.NullReferenceException: Object reference not set to an instance of an object.
   at MySqlConnector.MySqlDataReader.DisposeAsync(IOBehavior ioBehavior, CancellationToken cancellationToken) in /_/src/MySqlConnector/MySqlDataReader.cs:line 612
   at MySqlConnector.MySqlDataReader.Close() in /_/src/MySqlConnector/MySqlDataReader.cs:line 339
   at MySqlConnector.MySqlDataReader.Dispose(Boolean disposing) in /_/src/MySqlConnector/MySqlDataReader.cs:line 443
   at Dapper.SqlMapper.QueryAsync[T](IDbConnection cnn, Type effectiveType, CommandDefinition command) in /_/Dapper/SqlMapper.Async.cs:line 472

This only happens in v2.3.0 and later, but not in v2.2.7 and earlier.

bgrainger avatar Apr 21 '24 02:04 bgrainger

I think this is what's happening. I was able to reproduce the following two call stacks by deliberately injecting errors into the MySQL protocol being returned to MySqlConnector

Further investigation seems to indicate that CommandBehavior.CloseConnection (which Dapper sets if the connection isn't already open) is the problem. I'm able to reproduce these exceptions with this sample code (without injecting protocol failures):

Task.Run(async () =>
{
    await Task.Yield();
    using var connection = new MySqlConnection(csb.ConnectionString);
    await connection.OpenAsync();
    using var command = new MySqlCommand("SELECT 1;", connection);

    using var reader = await command.ExecuteReaderAsync(CommandBehavior.CloseConnection);
    return 1;
}));

For now, I can only get a repro with my in-memory MySQL protocol simulation, not with a real MySQL Server; I'm not sure why that is.

This code looks problematic: closing the connection will return the session to the pool and allow the MySqlDataReader to be picked up by another thread; then the current thread will clear values on it: https://github.com/mysql-net/MySqlConnector/blob/a7987abb4af4df4b58465aa8fafb293e89553a88/src/MySqlConnector/MySqlDataReader.cs#L640-L644.

bgrainger avatar Apr 21 '24 15:04 bgrainger

This breaks pretty reliably against a local MySql instance on my machine:

using MySqlConnector;
using System.Data;

var connectionString = Environment.GetEnvironmentVariable("CONNECTION_STRING");
async Task RunOnce(int instance)
{
    for (var index = 0; index < 1000; index++)
    {
        try
        {
            await using var connection = new MySqlConnection(connectionString);

            await connection.OpenAsync();

            using var command = connection.CreateCommand();

            command.CommandText = $"SELECT * FROM information_schema.columns LIMIT 1;";

            using var reader = await command.ExecuteReaderAsync(
                CommandBehavior.SequentialAccess | CommandBehavior.SingleResult | CommandBehavior.CloseConnection);

            connection.Close();

            await Task.Delay(10);
            await Task.Yield();
        }
        catch (Exception e)
        {
            Console.Error.WriteLine();
            Console.Error.WriteLine($"{instance}, {index}) " + e.ToString());
        }
    }
}

var tasks = new Task[Environment.ProcessorCount * 2];

for (var index = 0; index < tasks.Length; index++)
{
    var local = index;
    tasks[index] = Task.Run(() => RunOnce(local));
}

await Task.WhenAll(tasks);

jayrowe avatar Apr 21 '24 16:04 jayrowe

Thanks @jayrowe; that's a great test case. I was able to simplify the inner loop down to:

using var connection = new MySqlConnection(connectionString);
connection.Open();
using var command = connection.CreateCommand();
command.CommandText = $"SELECT 1;";
using var reader = command.ExecuteReader();
connection.Close();

await Task.Delay(10);

This crashes with 2.3.0 but not 2.2.7. The key part appears to be disposing the MySqlDataReader (via using) after the connection has been closed (which returns it to the pool). This ends up using the same object simultaneously from two different threads (which is the problem).

bgrainger avatar Apr 21 '24 19:04 bgrainger

We are experiencing the exact same issue when upgrading from 2.2.7 to 2.3.x. Thank you for fixing it.

Once the library is patched, I will also apply the patch and let you know the results.

ikpil avatar Apr 22 '24 07:04 ikpil

This is fixed in 2.3.7.

bgrainger avatar Apr 22 '24 14:04 bgrainger

Coming from the releases page, we're still getting "Packet received out-of-order" errors (likely in part related to having ProxySQL in front of our database), rolling back to 2.1.6 is an interim solution.

I'm not sure if this is already/still being tracked somewhere and don't immediately have the time to look into it, but thought it may be worth mentioning.

MySqlConnector.MySqlProtocolException
Packet received out-of-order. Expected 1; got 2.

at https://github.com/ppy/osu-queue-score-statistics/blob/33df310f3d34a56d38a3b354d91adf61c2b53ad4/osu.Server.Queues.ScoreStatisticsProcessor/ScoreStatisticsQueueProcessor.cs#L189

peppy avatar May 07 '24 03:05 peppy

I can also confirm that we hit this after upgrading to 2.3.7, packet out of order + NRE described above.

We do our own complex pooling however so it will take a while to get to the bottom of this. We definitely keep all of our connections open as long as possible, so we aren't intentionally disposing of our connections.

Will update if we can track it down.

alexr17 avatar Sep 23 '24 22:09 alexr17

Here is a repro of what we are doing/failing on:

public async Task TestUseReaderAfterDisposal2()
{
    MySqlDataReader firstReader = null;
    MySqlDataReader prevReader = null;
    MySqlConnection connection2 = new MySqlConnection
    {
        // false means pooling disabled
        ConnectionString = GetConnectionString(false),
    };
    
    // Keep the connection open
    await connection2.OpenAsync();
    for (int i = 0; i < 1000; ++i)
    {
        using (var command = connection2.CreateCommand())
        {
            command.CommandText = "SELECT * from mysql.user;";
            command.CommandType = CommandType.Text;

            // Execute command and process result
            using (var secondReader = await command.ExecuteReaderAsync())
            {
                if (firstReader == null)
                {
                    firstReader = secondReader;
                }
                else if (!firstReader.IsClosed || (prevReader?.IsClosed == false))
                {
                    // Throws in 2.3.0+ because the same reader instance is being used for the next command
                    throw new InvalidOperationException("Reader reports not closed after dispose.");
                }

                while (await secondReader.ReadAsync())
                {
                    secondReader.GetValue(0);
                }

                prevReader = secondReader;
            }
        }
    }
}

We are exchanging the reader before disposing it and checking if it is closed afterwards. I believe this breaks the pattern defined here: https://mysqlconnector.net/troubleshooting/connection-reuse/

My opinion is that the recycling of the reader object can lead to potential pitfalls, and I suspect that others may rely on a similar pattern to enable concurrent reads.

alexr17 avatar Sep 30 '24 16:09 alexr17

@alexr17 I think your repro boils down to this; is that correct?

MySqlDataReader usedReader; // will be used to capture the reader
using (var command = new MySqlCommand("select * from mysql.user", connection))
using (var reader = command.ExecuteReader())
{
	while (reader.Read())
	{
	}
	usedReader = reader;
	Console.WriteLine(usedReader.IsClosed); // False
}

Console.WriteLine(usedReader.IsClosed); // True

using (var command = new MySqlCommand("select * from mysql.user", connection))
using (var reader = command.ExecuteReader())
{
	Console.WriteLine(usedReader.IsClosed); // False, expected to stay True
	while (reader.Read())
	{
	}
}

This was noted in https://github.com/mysql-net/MySqlConnector/issues/1277 as a side-effect:

This could allow a user to start using a disposed reader without failure (once it's been handed out to a different caller). That currently fails with an ObjectDisposedException, so no existing code would be broken, but people could start writing new, broken code.

It is incorrect to use a MySqlDataReader after it's been disposed (by a using block). (Especially since you have to explicitly capture it and promote it outside of the using scope.) Is this reflective of your real production code?

bgrainger avatar Oct 08 '24 01:10 bgrainger

@bgrainger Yes this is what we were doing in prod, obviously have changed it now. It's weird that we never hit ObjectDisposedException, only NRE. Although we did upgrade from 2.1.0 straight to 2.3.7

alexr17 avatar Oct 14 '24 19:10 alexr17

There may have been code paths that didn't trigger ObjectDisposedException; perhaps that was an incorrect assumption on my part.

bgrainger avatar Oct 14 '24 20:10 bgrainger