views:

371

answers:

2

I am trying to run a task in the background checking a database for a number of records in a table, and if the number has changed since the last check, get those records, and do some work on them.

Using the following code, I'm getting a stack overflow after about two hours. The application is doing nothing during this time, just checking, and no jobs are being added to the database.

private Thread threadTask = null;
private int recordCount = 0;

private void threadTask_Start()
{
    if (threadTask == null) {
        threadTask = new Thread(taskCheck);
        threadTask.Start();
    }
}

private void taskCheck()
{
     int recordCountNew = GetDBRecordCound();
     if (recordCountNew != recordCount)
     {
         taskDo();
         recordCount = recordCountNew; // Reset the local count for the next loop
     }
     else
         Thread.Sleep(1000); // Give the thread a quick break

     taskCheck();          
}

private void taskDo()
{
    // get the top DB record and handle it
    // delete this record from the db
}

When it overflows, there are a ton of taskCheck() in the call stack. I'm guessing that taskCheck() never completes until taskCheck() completes, ergo overflow, and hence why they all remain in the stack. This is obviously not the right way to go about this, so what is?

+1  A: 

I'm assuming where you have task_Check(), you actually mean taskCheck() (or vice-versa). That is, you're calling the taskCheck() method recursively. Avoiding the discussion of the architecture here, you could remove that call and have threadTask_Start() repeat the process once in a while loop.

JP Alioto
Was a typo, fixed.
mattdwen
+9  A: 

You get a stack overflow because at the end of taskCheck, you call taskCheck again. You never exit the function taskCheck, you just end up calling more and more until your stack overflows. What you should do is have a while loop in taskCheck instead:

private void taskCheck()
{
   while(true)
   {
       int recordCountNew = GetDBRecordCound();
       if (recordCountNew != recordCount)
       {
           taskDo();
           recordCount = recordCountNew; // Reset the local count for the next loop
       }
       else
           Thread.Sleep(1000); // Give the thread a quick break
   }
}
MedicineMan
He should also have a try{}catch(ThreadAbortException){}finally{} block.
Adam Robinson
good point. If an exception is thrown (database call?) the flow of execution will exit the while loop and the while loop will stop executing. I usually inform the user in a message log or pop up if this occurs. Alternatives?
MedicineMan
There is other database handling going on, which I'll use to control the 'true' value if there is a problem.
mattdwen