views:

519

answers:

3

I have an application that has two threads.

The first one (the main thread) that captures the data using socket and update DataTables

The second Inserts the DataTables into the database.

The application works fine but when it closes, the main thread finishes reading the data and call Abort method in the second thread, which may be inserting into database and this leads to inconsistent data.

Currently I am using the following solution to overcome "aborting during insertion"

EDIT: After the powerful answers I changed the code

void MainThread()
{
     while(Read())
     {
        //Read Data through socket
        try
        {
           //Wait on Mutex1
           //Update Tables
        }
        finally
        {
          //Release Mutex1
        }
     }
   _isrunning = false;
   _secondThread.Join();
}
void SecondThread()
{
     while(_isrunning)
     {
        try
        {
           //Wait on Mutex1
           //Insert Tables into Database using transactions
        }
        finally
        {
           //Release Mutex1           
        }
     }
}
+8  A: 

As long as both threads are not marked as background threads, the app will keep running until both threads exit. So really, all you need to do is to get each thread separately to exit cleanly. In the case of the thread that writes to the database, this may mean exhausting a producer/consumer queue and checking a flag to exit.

I showed a suitable producer/consumer queue here - the worker would just be:

void WriterLoop() {
    SomeWorkItem item; // could be a `DataTable` or similar
    while(queue.TryDequeue(out item)) {
        // process item
    }
    // queue is empty and has been closed; all done, so exit...
}


Here's a full example based on SizeQueue<> - note that the process doesn't exit until the reader and writer have exited cleanly. If you don't want to have to drain the queue (i.e. you want to exit sooner, and forget any pending work), then fine - add an extra (volatile) flag somewhere.

static class Program {
    static void Write(object message) {
        Console.WriteLine(Thread.CurrentThread.Name + ": " + message);
    }
    static void Main() {
        Thread.CurrentThread.Name = "Reader";
        Thread writer = new Thread(WriterLoop);
        writer.Name = "Writer";
        var queue = new SizeQueue<int>(100);
        writer.Start(queue);
        // reader loop - note this can run parallel
        // to the writer
        for (int i = 0; i < 100; i++) {
            if (i % 10 == 9) Write(i);
            queue.Enqueue(i);
            Thread.Sleep(5); // pretend it takes time
        }
        queue.Close();
        Write("exiting");
    }
    static void WriterLoop(object state) {
        var queue = (SizeQueue<int>)state;
        int i;
        while (queue.TryDequeue(out i)) {
            if(i%10==9) Write(i);
            Thread.Sleep(10); // pretend it takes time
        }
        Write("exiting");
    }
}
Marc Gravell
Still an app crash should not cause db corruption ...
Sam Saffron
True, individual updates should be ACID - but the crash here is self inflicted by calling Abort.
Marc Gravell
there is an edge case where you may want to inflict a crash, eg. in a window service you have an SLA you want to meet when shutting down an app. If for some reason a call is "stuck" at the db due to blocking and you can not signal it, you may consider aborting
Sam Saffron
+1. Every thread should be totally and wholly responsible for its own lifetime. By all means have other threads allowed to *suggest* that a thread should shut down but the if and the when should be done by the thread in question (this includes suspending and resuming threads as well). This way, you will never get deadlock or corruption situations because there's no uncontrolled access to the threads activities.
paxdiablo
@Sam - indeed, the only time I'd consider aborting a thread is to kill a sick process. And by the time you are considering a thread-abort, I might also consider a separate exe and Process.Start, so that I can kill the entire process if I need. Sometimes, something that brutal is the only guaranteed way....
Marc Gravell
ok, consider my code , the first thread is the main thread and the second thread is background thread, so without doing abort the application will kill the background thread that may lead to db corruption
Ahmed Said
@Ahmed, when the application wants to shut down, it should send a "message" to the other thread then wait for it to exit of its own accord before shutting down. The other thread should be periodically watching for this message and acting on it in a timely fashion. By message, I mean event in an event pump, mutex-protected variable or even a non-mutexed boolean read only by the other thread. If the other thread is designed correctly, the main thread won't have to wait around long.
paxdiablo
@Pax, but my background thread is working forever (while(true), so I do not think that when the main thread is shutting down it will close the background thread gracefully (I think it will kill it)
Ahmed Said
@Marc, you see my answer, the big fat root problem of all this mess is that @Ahemed is using mutexes instead of db transactions to manage db consistency. Something that is wrong %99.999 of the time.
Sam Saffron
so... change the code! Either make it not a background thread, or introduce a shutdown variable (i.e. don't just while(true)). If you don't change anything, your system will keep acting the same way...
Marc Gravell
@Sam - I agree fully. Mutex dost not a transaction make.
Marc Gravell
@sam, I am using mutex for two things one for updating datatables , second thing is to prevent thread.abort during db insertion
Ahmed Said
(responded to the point above on the original post)
Marc Gravell
@sam, I know this is not good way to handle the problem so I asked for better, but till now the links provided say "do not use abort" but not say why?
Ahmed Said
+6  A: 

Assuming "call abort method" means aborting the thread using Thread.Abort. Don't do that.

You are effectively crashing your app. There are plenty cleaner ways of doing this with Monitors.

Nonetheless, you should not be getting inconsistent data in your DB when your app crashes that's why you have DB transactions which have the ACID properties.

VERY IMPORTANT EDIT You said: you do not use transactions for performance reasons, and instead use mutexes. This is WRONG on quite a few levels. Firstly, transactions can make certain operations faster, for example try inserting 10 rows into a table, try it again within a transaction, the transaction version will be faster. Secondly, what happens when/if your app crashes, do you corrupt your DB? What happens when multiple instances of your app are running? Or while you run reports against your DB in query analyzer?

Sam Saffron
I do not want to use transaction (for performance wise), so without using mutex and thread.abort the application'll lead to inconsistent records
Ahmed Said
After the comment and your edit, I wish I could upvote this a second time.
Marc Gravell
I have only single instance of my application, and I dont use transaction during insertion as the insert method runs every second and it should not take more than second
Ahmed Said
+3  A: 

Your mutex wait should involve a timeout. Each thread's outer loop can check for a 'please close now' flag. To shut down, set the 'please close now' flag for each thread, then use 'join' to wait for each thread to finish.

Andrew
wonderful answer, simple and effective
Ahmed Said