You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
System.InvalidOperationException in FirebirdSql.Data.FirebirdClient #178
This code succeeds in raw ADO.net Executereader(), but fails in dapper Executereader().
IDataReader.Read() throws a System.InvalidOperationException exception with the message:
Invalid attempt of read when the reader is closed.
usingSystem;usingSystem.Data;usingDapper;usingFirebirdSql.Data.FirebirdClient;namespaceConsoleApplication1{classProgram{staticvoidMain(string[]args){varcs=@"initial catalog=localhost:database;user id=SYSDBA;password=masterkey";varsql=@"select count(*) from user";varcon=newFbConnection(cs);con.Open();// raw ADO.netvarsqlCmd=newFbCommand(sql,con);IDataReaderreader1=sqlCmd.ExecuteReader();while(reader1.Read()){Console.WriteLine(reader1[0].ToString());// output is 10}// dappervarreader2=con.ExecuteReader(sql);while(reader2.Read())// System.InvalidOperationException. "Invalid attempt of read when the reader is closed."{Console.WriteLine(reader2[0].ToString());}Console.Read();}}}
Stack trace:
System.InvalidOperationException was unhandled by user code.
HResult=-2146233079
Message=Invalid attempt of read when the reader is closed.
Source=FirebirdSql.Data.FirebirdClient
StackTrace:
at FirebirdSql.Data.FirebirdClient.FbDataReader.CheckState()
at FirebirdSql.Data.FirebirdClient.FbDataReader.Read()
at ConsoleApplication2.Program.Main(String[] args) at c:\Users\aoki\Desktop\ConsoleApplication2\ConsoleApplication2\Program.cs:行 33
at System.AppDomain._nExecuteAssembly(RuntimeAssembly assembly, String[] args)
at System.AppDomain.ExecuteAssembly(String assemblyFile, Evidence assemblySecurity, String[] args)
at Microsoft.VisualStudio.HostingProcess.HostProc.RunUsersAssembly()
at System.Threading.ThreadHelper.ThreadStart_Context(Object state)
at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state, Boolean preserveSyncCtx)
at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
at System.Threading.ThreadHelper.ThreadStart()
InnerException:
Interesting; I have a repro; this is actually caused by the disposal of the DbCommand object inside ExecuteReaderImpl. I am tempted to say that this is actually a bug in the firebird ADO.NET implementation - disposing the command does not usually kill the reader. I'm going to float this past the provider authors, to see what they think.
Hi Marc. I'm the one responsible for FirebirdClient. Thanks for the patch (BTW there's a mirror on GitHub, which is more pleasant to work with, IMO), sadly it's wrong.
First the Dispose method itself also calls this.activeReader = null; and this.statement = null;, so just skipping closing reader in Release is not enough.
Unfortunately as the code is now the reader itself needs command to fetch records. Initially only few are fetched (whatever fits the first response buffer) and then more is fetched as the buffer is processed. The command (actually something called "statement") holds the resources on server and handles all that and command keeps all the references etc. I know it's not a best design, but that's how it started and is now. :(
I wish to have some quick fix, but probably only a big rewrite would work. Could you not dispose the command before reader on your side?
Thanks for the reply. If the nulls were the only issue, it could of course
be tweaked to set those to null.
Re the command vs reader: it kinds sounds like at this point ownership has
passed to the reader, and thus the correct place to manage that is in the
reader, not the command. I fully sympathise, however, as one library author
to another, that this type of change is not trivial. Ultimately, the vexing
thing is that it works fine for every other ADO.NET provider.
Some of the APIs : yes, I could change the implementation to keep the
command action for longer. Not convenient, but possible. However, there is
also an API on dapper that returns the reader to the caller without ever
showing them the command. If I don't dispose the command, no-one ever will
(no-one else ever sees it). I can't wrap the reader, as the caller might
be casting to the provider-specific version for: reasons.
I guess I can patch up dapper to get by as best it can, but long term, I do
still think this is something that belongs in the reader...
Marc
On 6 Oct 2014 13:20, "Jiri {x2} Cincura" [email protected] wrote:
Hi Marc. I'm the one responsible for FirebirdClient. Thanks for the patch
(BTW there's a mirror on GitHub https://github.com/cincuranet/NETProvider, which is more pleasant to
work with, IMO), sadly it's wrong.
First the Dispose method itself also calls this.activeReader = null; and this.statement
= null;, so just skipping closing reader in Release is not enough.
Unfortunately as the code is now the reader itself needs command to fetch
records. Initially only few are fetched (whatever fits the first response
buffer) and then more is fetched as the buffer is processed. The command
(actually something called "statement") holds the resources on server and
handles all that and command keeps all the references etc. I know it's not
a best design, but that's how it started and is now. :(
I wish to have some quick fix, but probably only a big rewrite would work.
Could you not dispose the command before reader on your side?
—
Reply to this email directly or view it on GitHub #178 (comment)
.
Re the command vs reader: it kinds sounds like at this point ownership has
passed to the reader, and thus the correct place to manage that is in the
reader, not the command.
I agree. You can create (or I'll later) a ticket at tracker.firebirdsql.org. There's a couple of bad pieces of code there. I'm slowly trying to remove them.
I can't wrap the reader, as the caller might
be casting to the provider-specific version for: reasons.
I see. Probably only overriding casting operator would work. But that's hassle.
Overriding the casting simply operator isn't viable:
the return type is IDataReader; you can't add a custom casting operator
to an arbitrary interface
the code that would need to do this doesn't know the type of reader, so
can't expose a custom conversion operator to that type - the reader comes
from ADO.NET; dapper does not include assembly references for firebird,
oracle, mysql, etc...
Re the command vs reader: it kinds sounds like at this point ownership has
passed to the reader, and thus the correct place to manage that is in the
reader, not the command.
I agree. You can create (or I'll later) a ticket at
tracker.firebirdsql.org. There's a couple of bad pieces of code there.
I'm slowly trying to remove them.
I can't wrap the reader, as the caller might
be casting to the provider-specific version for: reasons.
I see. Probably only overriding casting operator would work. But that's
hassle.
—
Reply to this email directly or view it on GitHub #178 (comment)
.
This has been "fixed" (worked around) in the most recent dapper commit - which wraps the reader such that the command's lifetime is linked to the reader. Not ideal, but it works.
Activity
mgravell commentedon Sep 22, 2014
Interesting; I have a repro; this is actually caused by the disposal of the DbCommand object inside ExecuteReaderImpl. I am tempted to say that this is actually a bug in the firebird ADO.NET implementation - disposing the command does not usually kill the reader. I'm going to float this past the provider authors, to see what they think.
mgravell commentedon Sep 22, 2014
I have submitted a pull request to the provider (https://sourceforge.net/p/firebird/NETProvider/merge-requests/2/); there isn't much more I can do... I'm reluctant to not dispose the commands. Thoughts?
guntama commentedon Sep 22, 2014
Thank you for dealing with this matter.
I agree with you . I also think this is a problem in FirebirdClient.
mgravell commentedon Sep 22, 2014
cheers; I can't force firebird to take the PR, but I've done what I can
cincuranet commentedon Oct 6, 2014
Hi Marc. I'm the one responsible for FirebirdClient. Thanks for the patch (BTW there's a mirror on GitHub, which is more pleasant to work with, IMO), sadly it's wrong.
First the Dispose method itself also calls
this.activeReader = null;andthis.statement = null;, so just skipping closing reader inReleaseis not enough.Unfortunately as the code is now the reader itself needs command to fetch records. Initially only few are fetched (whatever fits the first response buffer) and then more is fetched as the buffer is processed. The command (actually something called "statement") holds the resources on server and handles all that and command keeps all the references etc. I know it's not a best design, but that's how it started and is now. :(
I wish to have some quick fix, but probably only a big rewrite would work. Could you not dispose the command before reader on your side?
mgravell commentedon Oct 6, 2014
Thanks for the reply. If the nulls were the only issue, it could of course
be tweaked to set those to null.
Re the command vs reader: it kinds sounds like at this point ownership has
passed to the reader, and thus the correct place to manage that is in the
reader, not the command. I fully sympathise, however, as one library author
to another, that this type of change is not trivial. Ultimately, the vexing
thing is that it works fine for every other ADO.NET provider.
Some of the APIs : yes, I could change the implementation to keep the
command action for longer. Not convenient, but possible. However, there is
also an API on dapper that returns the reader to the caller without ever
showing them the command. If I don't dispose the command, no-one ever will
(no-one else ever sees it). I can't wrap the reader, as the caller might
be casting to the provider-specific version for: reasons.
I guess I can patch up dapper to get by as best it can, but long term, I do
still think this is something that belongs in the reader...
Marc
On 6 Oct 2014 13:20, "Jiri {x2} Cincura" [email protected] wrote:
cincuranet commentedon Oct 7, 2014
I agree. You can create (or I'll later) a ticket at tracker.firebirdsql.org. There's a couple of bad pieces of code there. I'm slowly trying to remove them.
I see. Probably only overriding casting operator would work. But that's hassle.
mgravell commentedon Oct 7, 2014
Overriding the casting simply operator isn't viable:
to an arbitrary interface
can't expose a custom conversion operator to that type - the reader comes
from ADO.NET; dapper does not include assembly references for firebird,
oracle, mysql, etc...
On 7 October 2014 05:49, Jiri {x2} Cincura [email protected] wrote:
Regards,
Marc
cincuranet commentedon Oct 7, 2014
OK, well. I think currently the
FbDataReaderis out of business. Either I'll rework it or maybe somebody will contribute.mgravell commentedon Oct 7, 2014
"out of business"?
On 7 October 2014 08:34, Jiri {x2} Cincura [email protected] wrote:
Regards,
Marc
cincuranet commentedon Oct 7, 2014
Like that it will not work with Dapper correctly.
mgravell commentedon Oct 7, 2014
Since dapper is just using ADO.NET here, it could be that it fails in a
number of other tools that use ADO.NET; but... it is what it is.
On 7 October 2014 09:23, Jiri {x2} Cincura [email protected] wrote:
Regards,
Marc
mgravell commentedon Oct 14, 2014
This has been "fixed" (worked around) in the most recent dapper commit - which wraps the reader such that the command's lifetime is linked to the reader. Not ideal, but it works.