Past several days I was investigating some very interesting multi-threading issue. Luckily I was able to reproduce it in about 80% of cases so I could start debugging right away. The problem is the code base is big and I didn’t have an idea why this happens. But first a little background:
Service
The hosting service is WCF application hosted in IIS. The underlying data is fetched from different MS SQL databases depending on the input. Basically depending on what data is needed one or more databases are queried using stored procedures. To optimize performance since those calls are heavy are parallelized.
The issue
Randomly were logged exceptions of the type “System.InvalidOperationException: ExecuteReader requires an open and available Connection. The connection's current state is connecting/closed/reading.” (state is modified by me since all kind of states are passed there). When calls are made sequential the issue disappears but the overall time for operation increases a lot and thus such fix is not acceptable.
The story
Since the code contained some generics, lambda, loops to parallelize calls (it is .NET 3.5 application which doesn’t use Parallel Extensions). Thus where to start from?!
Of course – #1 try – repro in simple application. Unfortunately didn’t work(keep reading to see why
)
Next attempt is to start changing code so if there are some evil caveats (like this) to be avoided in time. So I killed generics.. Result is same 
Next is removing lambda and replacing with methods and delegates… Result is still same.
Replace threading from BeginInvoke() and EndInvoke() to ThreadPool.QueueWorkItem()… Result is still same.
Change threading to custom Thread.Start() and thread synchronization… Result is still same.
Twisted threading here and there… still same
Well… it was time to start cutting code: Cut DAO layer… no change. Cut infrastructure code for accepting calls.. still same. This greatly helped me to narrow the code too look at and I put working simple application inside the service. Strange but this worked with no issues. This could mean only one thing – I am getting very close. Then I started building up this simple method to look like the problem one. And then I spotted the problem – small, tiny problem that cause a lot of headache.
The problem
Examine the code below. All you need is access to a SQL database. Connection string is put in code for convenience.
1: class Program {
2:
3: class CommandData {
4: public Action Command { get; set; }
5: public IAsyncResult AsyncResult { get; set; }
6: }
7:
8: static string connectionString = @"data source=.;initial catalog=AdventureWorks;Integrated security=true;";
9: static int commandNumber = 20;
10:
11: static void Main(string[] args) {
12:
13: var commands = new List<CommandData>(commandNumber);
14:
15: SqlCommand cmd = new SqlCommand("SELECT DB_NAME()");
16:
17: for (int i = 0; i < commandNumber; i++) {
18: Action act = () => {
19: ExecuteCommand(cmd);
20: };
21:
22: commands.Add(new CommandData { Command=act });
23: }
24:
25: for (int i = 0; i < commands.Count; i++) {
26: var item = commands[i];
27: item.AsyncResult = item.Command.BeginInvoke(null, null);
28: }
29:
30: for (int i = 0; i < commands.Count; i++) {
31: var item = commands[i];
32: item.Command.EndInvoke(item.AsyncResult);
33: }
34: }
35:
36: public static DataSet ExecuteCommand(SqlCommand cmd) {
37: var result = new DataSet();
38: var conn = new SqlConnection(connectionString);
39:
40: try {
41: cmd.Connection = conn;
42: var da = new SqlDataAdapter(cmd);
43: da.Fill(result);
44: Console.WriteLine("\t{0}", result.Tables.Count);
45: Console.WriteLine();
46: } catch (Exception ex) {
47: Console.WriteLine(ex.ToString());
48: //throw;
49: } finally {
50: conn.Close();
51: }
52:
53: return result;
54: }
55: }
Run this code and you’ll see the error.
Resolution
The resolution is simple – just replace for loop body on lines 18 to 20 with following snipped:
1: SqlCommand cmd2 = cmd.Clone();
2: Action act = () => {
3: ExecuteCommand(cmd2);
4: };
Run it now and should work fine.
The problem is that same instance of SqlCommand is used in all threads and different connections are assinged to it and not all of them are open as the exception suggests. But the true problem is concurrency on SqlCommand.
Conclusions
- Inside the loop try not to use variables declared outside the loop.
- Lambda is safe – using delegates has same issues – Mind this problem
- Watch your repro application to be as much possible close to original and yet simple. If needed ask a teammate to have a look.
I hope you enjoyed this long read
Download the code.