-
Notifications
You must be signed in to change notification settings - Fork 337
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
NullReferenceException in MySqlConnector.MySqlDataReader.ActivateResultSet #1459
Comments
Can you provide a self-contained example that reproduces the problem? A |
Related: #1427 (comment) |
thank you very much. I will study the code. (This is old code, converted from Oracle MySQL driver.) |
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:
Sometimes there is a null reference during Dispose:
Sometimes there is an null reference during ActivateResultSet:
There is a packet out-of-order error, too:
I don't have a usable stack trace for the last one, since it is wrapped up in an AggregateException |
This call stack doesn't "look right". I wonder if there's an earlier exception that's being thrown, but this |
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 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. |
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 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? |
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:
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 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. |
Thanks for the update. That code looks pretty "bog standard", as you said. Does |
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. |
I see you're passing in |
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 |
+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. |
Is anyone who is experiencing this problem able to provide a self-contained example that reproduces the exception? |
One major change in 2.3.5 was #1277 which recycles
The only thing that could be
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 This value is set to |
Another thing that might help is providing trace-level logging leading up to the exception. |
Hey. Context: The downgrade MySqlConnector to 2.3.1 does not help. Here is a Stack trace from MySqlConnector v2.3.1:
Connections are not disposed explicitly, "await using" used for that.
|
I created a test program to issue a large number of simultaneous queries to MySQL Server with Dapper (using
After several dozen runs, I never saw a At this point I think I need someone who's experiencing the problem to provide either a repro or detailed logs. |
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
This only happens in v2.3.0 and later, but not in v2.2.7 and earlier. |
Further investigation seems to indicate that 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 MySqlConnector/src/MySqlConnector/MySqlDataReader.cs Lines 640 to 644 in a7987ab
|
This breaks pretty reliably against a local MySql instance on my machine:
|
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 |
We are experiencing the exact same issue when upgrading from 2.2.7 to 2.3.x. Once the library is patched, I will also apply the patch and let you know the results. |
This is fixed in 2.3.7. |
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.
|
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. |
Here is a repro of what we are doing/failing on:
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 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 #1277 as a side-effect:
It is incorrect to use a |
@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 |
There may have been code paths that didn't trigger |
An optimization introduced in v2.3.0 started recycling
MySqlDataReader
instances: #1277.If client code held onto the
MySqlDataReader
and disposed it after the associatedMySqlConnection
had been returned to the connection pool, this could cause that connection to be disposed while it was being used by another thread, leading to unpredictableNullReferenceException
s.While many access patterns didn't trigger this bug (e.g., no problems have been reported by users of Pomelo.EntityFrameworkCore.MySql), using
CommandBehavior.CloseConnection
or Dapper'sQueryAsync<T>
made it more likely to happen.Original bug report below.
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
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
The text was updated successfully, but these errors were encountered: