Windows Service is Running but not executing code

Question

We have windows service which is running fine untill any exceptions occured in the process. It contains two Threads (GenerateInvoice and GenerateReport). These threads are getting blocked and results in DeadLock like situation mostly when there is high CPU usage on our DataBase server.

We have done some changes in code to handle such situations like added while condition below code but still it is not working. Below is the OnStart() method of service:

protected override void OnStart(string[] args)
{
    try
    {
        log.Debug("Starting Invoice Generation Service");
        _thread = new Thread(new ThreadStart((new GenerateInvoice()).Process));
        _thread.IsBackground = true;
        _thread.Start();

        _reportThread = new Thread(new ThreadStart((new GenerateReport()).Process));
        _reportThread.IsBackground = true;
        _reportThread.Start();
    }
    catch (Exception ex)
    {
        log.Error("Error in Invoice Generation Service:", ex);
    }
}

Here is the processing code of first thread: GenerateInvoice

public void Process()
{
    while (isProcessActive) 
    {
        try
        {
            DBBilling obj = new DBBilling();
            DataTable dtInvoiceID = obj.readData(@"SELECT * FROM (SELECT ird.BillByType, ird.InvoiceID, ir.BeginDate, ir.EndDate, ir.SendToQB, ir.SendEmail, 
                i.ARAccountID, i.ARAccountHotelID, i.invoiceNumber,i.[STATUS],UPDATETIME,row_number() over (PARTITION BY ird.INVOICEID ORDER BY UPDATETIME DESC) AS row_number
                FROM Invoices i JOIN  InvoicesRunRequestDetails ird ON ird.InvoiceID=i.InvoiceID 
                JOIN InvoicesRunRequest ir ON ird.RequestID = ir.RequestID
                Where i.[STATUS] = 'PENDING') AS rows
                WHERE ROW_NUMBER=1 ORDER BY UPDATETIME");

            processCounter = 0;

            #region process
            if (dtInvoiceID != null && dtInvoiceID.Rows.Count > 0)
            {
              //some code here..
            }
            #endregion
        }
        catch (Exception ex)        //Mantis 1486 : WEBPMS1 Disk Space : 10 Aug 2016
        {
            log.ErrorFormat("Generate Invoice -> Process -> InnLink Billing Execute Query Exception. Error={0}", ex);
            if(DBBilling.dbConnTimeoutErrorMessage.Any(ex.Message.Contains))
            {
                processCounter++;
                if (processCounter >= 1) //Need to change to 25 after Problem Solve
                {
                    isProcessActive = false;
                    log.ErrorFormat("Generate Invoice -> Process -> RunInvoice Service exiting loop"); //From here control is not going back                
                }
                else 
                    System.Threading.Thread.Sleep(5000);    //Sleep for 5 Sec
            }
        }                
    }        
}

Processing of Second Thread i.e. GenerateReport code:

public void Process()
{
    AppSettingsReader ar = new AppSettingsReader();
    string constr = (string)ar.GetValue("BillingDB", typeof(string));
    SqlConnection con = new SqlConnection(constr);
    while (isProcessActive) 
    {
        try
        {
            DBBilling obj = new DBBilling();
            DataTable dtReportRunID = obj.readData(@"SELECT ReportRunID,MonYear, BeginDate, EndDate FROM ReportRunRequest 
                Where [STATUS] = 'PENDING' ORDER BY ReportRunID");
            processCounter = 0;

            if (dtReportRunID != null && dtReportRunID.Rows.Count > 0)
            {
                //some code here..
            }
        }
        catch (Exception ex)        //Mantis 1486 : WEBPMS1 Disk Space : 10 Aug 2016
        {
            log.ErrorFormat("Generate Report -> Process -> InnLink Billing Execute Query Exception. Error={0}", ex);
            if (DBBilling.dbConnTimeoutErrorMessage.Any(ex.Message.Contains))
            {
                processCounter++;
                if (processCounter >= 1) //Need to change to 25 after Problem Solve
                {
                    isProcessActive = false;
                    log.ErrorFormat("Generate Report -> Process -> RunInvoice Service Exiting loop");  //From here control is not going back                             
                } 
                else
                    System.Threading.Thread.Sleep(5000);    //Sleep for 5 Sec
            }
        }
    }
}

What possible solution to avoid such conditions?

Answer 1

The way to avoid it is to either lock every access to a global variable, or not to use global variables.

here is one obvious example

DBBilling.dbConnTimeoutErrorMessage.Any(ex.Message.Contains)

dbConnTimeoutErrorMessage is a static field that is being used from two different threads and I assume is not thread safe, surround access to it with a

lock(locObj)
{
   // access to dbConnTimeoutErrorMessage
}

I am gonna go ahead and guess that log is also a global variable. Perhaps maybe even isProcessActive or processCounter.

I am guessing there is more in those comments - make sure your code is threadsafe before using it with two different threads.

I doubt locking access to what I said will fix your problem, but I guess your lack of threadsafe programming in these is a symptom to not using lock when it is needed. The secret is to lock every access to a global context, and just that.

Answer 2

What i suggest is to use Timer instead of infinite loop and as mentioned earlier in other answere you need some kind of synchronization. First of all, you need to implement your variables which used in different threads as follows (i don't know exactly definitions of your variables, but main idea is to use volatile keyword in your case):

public static volatile bool isProcessActive;
public static volatile int proccessCounter;

volatile keyword switches off the compiler optimizations for using variable in one thread. It means that your variables now are thread safe.

Next you need to use neither System.Threading.Timer or System.Timers.Timer. I will use in my example second one.

public sealed class GenerateInvoice :
{
    protected const int timerInterval = 1000; // define here interval between ticks

    protected Timer timer = new Timer(timerInterval); // creating timer

    public GenerateInvoice()
    {
        timer.Elapsed += Timer_Elapsed;     
    }

    public void Start()
    {
        timer.Start();
    }

    public void Stop()
    {
        timer.Stop();
    }

    public void Timer_Elapsed(object sender, ElapsedEventArgs e)
    {       
        try
        {
            DBBilling obj = new DBBilling();
            DataTable dtInvoiceID = obj.readData(@"SELECT * FROM (SELECT ird.BillByType, ird.InvoiceID, ir.BeginDate, ir.EndDate, ir.SendToQB, ir.SendEmail, 
                i.ARAccountID, i.ARAccountHotelID, i.invoiceNumber,i.[STATUS],UPDATETIME,row_number() over (PARTITION BY ird.INVOICEID ORDER BY UPDATETIME DESC) AS row_number
                FROM Invoices i JOIN  InvoicesRunRequestDetails ird ON ird.InvoiceID=i.InvoiceID 
                JOIN InvoicesRunRequest ir ON ird.RequestID = ir.RequestID
                Where i.[STATUS] = 'PENDING') AS rows
                WHERE ROW_NUMBER=1 ORDER BY UPDATETIME");

            processCounter = 0;

            #region process
            if (dtInvoiceID != null && dtInvoiceID.Rows.Count > 0)
            {
              //some code here..
            }
            #endregion
        }
        catch (Exception ex)        //Mantis 1486 : WEBPMS1 Disk Space : 10 Aug 2016
        {
            log.ErrorFormat("Generate Invoice -> Process -> InnLink Billing Execute Query Exception. Error={0}", ex);
            if(DBBilling.dbConnTimeoutErrorMessage.Any(ex.Message.Contains))
            {
                processCounter++;
                if (processCounter >= 1) //Need to change to 25 after Problem Solve
                {
                    isProcessActive = false;
                    // supposing that log is a reference type and one of the solutions can be using lock
                    // in that case only one thread at the moment will call log.ErrorFormat
                    // but better to make synchronization stuff unside logger
                    lock (log)
                        log.ErrorFormat("Generate Invoice -> Process -> RunInvoice Service exiting loop"); //From here control is not going back                
                }
                else 
                    // if you need here some kind of execution sleep 
                    // here you can stop timer, change it interval and run again
                    // it's better than use Thread.Sleep

                    // System.Threading.Thread.Sleep(5000);    //Sleep for 5 Sec
            }
        }                     
    }
}

Use the same approach for the GenerateReport to make Timer-based.

And, finally, you need to change your OnStart and OnStop methods something like so:

protected GenerateInvoice generateInvoice;
protected GenerateReport generateReport;

protected override void OnStart(string[] args)
{
    // all exception handling should be inside class

    log.Debug("Starting Invoice Generation Service");

    generateInvoice = new GenerateInvoice();
    generateInvoice.Start();

    generateReport = new GenerateReport();
    generateReport.Start();
}

protected override void OnStop()
{
    generateInvoice.Stop();
    generateReport.Stop();
}