Re: [Firebird-net-provider] Deadlock problem (possible: DNET-382)
On 29-9-2011 17:37, Scott Morgan wrote: On 09/29/2011 04:20 PM, Mark Rotteveel wrote: The solution would at minimum require that FbCommand.Dispose sync in the exact same order as Rollback (so first database.syncObject, and then GdsStatement) (or that Rollback use the exact same order as in Dispose). Then it is impossible for deadlock to occur (assuming that the transaction isn't locked further on the Dispose method). Easier said than done, the locks are made at different points in the code, not all in the one function. See the call stacks at the end of the original message. True, but making sure that you always lock resources in the same order, is the only sure way to prevent deadlocks. Worth noting also that there are a lot of 'lock(this)' entries, which is something MS recommends against in it's docs. I know next to nothing about .NET, I just follow this list as I recently started working on Jaybird and things posted here might have similar problems or applications in Jaybird :) -- Mark Rotteveel -- All of the data generated in your IT infrastructure is seriously valuable. Why? It contains a definitive record of application performance, security threats, fraudulent activity, and more. Splunk takes this data and makes sense of it. IT sense. And common sense. http://p.sf.net/sfu/splunk-d2dcopy2 ___ Firebird-net-provider mailing list Firebird-net-provider@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/firebird-net-provider
Re: [Firebird-net-provider] Deadlock problem (possible: DNET-382)
On 09/30/2011 12:59 PM, Mark Rotteveel wrote:
True, but making sure that you always lock resources in the same order,
is the only sure way to prevent deadlocks.
I think I see the problem:
In Version10/GdsTransaction.cs (copy/paste from the Sourceforge SVN
browser, so excuse the formatting)
186 public void Rollback()
187 {
188 this.CheckTransactionState();
189
190 lock (this.database.SyncObject)
191 {
192 try
193 {
194 this.database.Write(IscCodes.op_rollback);
195 this.database.Write(this.handle);
196 this.database.Flush();
197
198 this.database.ReadResponse();
199
200 this.database.TransactionCount--;
201
202 if (this.Update != null)
203 {
204 this.Update(this, new EventArgs());
205 }
206
207 this.state = TransactionState.NoTransaction;
208 }
209 catch (IOException)
210 {
211 throw new IscException(IscCodes.isc_net_read_err);
212 }
213 }
214 }
The SyncObject lock is to protect the calls to this.database.Write etc.
(lines 194-200) but it also covers the transaction's state change (lines
202-207) which it shouldn't and is the point the deadlock occurs.
Something like this may be better:
public void Rollback()
{
this.CheckTransactionState();
lock (aNewPrivateTransactionLockObject)
{
try
{
// Make sure this sequence of actions on the DB object
isn't broken
lock (this.database.SyncObject)
{
this.database.Write(IscCodes.op_rollback);
this.database.Write(this.handle);
this.database.Flush();
this.database.ReadResponse();
this.database.TransactionCount--;
}
if (this.Update != null)
{
this.Update(this, new EventArgs());
}
this.state = TransactionState.NoTransaction;
}
...etc...
}
}
Similar changes would need to be made to the other functions in this
class (Commit, RollbackRetaining, etc...)
Scott
--
All of the data generated in your IT infrastructure is seriously valuable.
Why? It contains a definitive record of application performance, security
threats, fraudulent activity, and more. Splunk takes this data and makes
sense of it. IT sense. And common sense.
http://p.sf.net/sfu/splunk-d2dcopy2
___
Firebird-net-provider mailing list
Firebird-net-provider@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/firebird-net-provider
