search

mysql-net / MySqlConnector

MySQL Connector for .NET
https://mysqlconnector.net
MIT License
1.39k stars 337 forks source link

Recycle `MySqlDataReader` instances #1277

Open bgrainger opened 1 year ago

bgrainger commented 1 year ago

Since there can only be one active MySqlDataReader per MySqlConnection, we could create just one instance and recycle it when it's disposed.

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.

Related: https://github.com/npgsql/npgsql/issues/1649, https://github.com/mysql-net/MySqlConnector/issues/1264.

bgrainger commented 6 months ago

The implementation of this in v2.3.0 breaks the following code (which is an unexpected pattern, but not "wrong"):

using var connection = ...
using var reader = ...
connection.Close();
// reader disposed afterwards

https://github.com/mysql-net/MySqlConnector/issues/1459#issuecomment-2068168122

It also seems to break Dapper's QueryAsync<>, which isn't using that exact pattern (AFAICT) but may trigger some code path that exposes a race condition in MySqlConnector.

The solution may be to make MySqlDataReader as slim as possible (maybe just referencing a ResultSet and MySqlCommand?), and pool and recycle the more heavy-weight ResultSet object.