5

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           
        }
     }
}
Ahmed
  • 7,148
  • 12
  • 57
  • 96
  • 5
    So.. don't call "Abort"! That is not a good idea. Ever. – Marc Gravell Jun 28 '09 at 09:51
  • 1
    Re your comment "I do not want to use transaction (for performance wise), so without using mutex and thread.abort the application'll lead to inconsistent records" - I have to second Sam; that is just crazy. Most databases are fully optimized for transactions (indeed, often only a rollback carries additional cost). It doesn't matter how fast your app can run if it corrupts state. Get it working robustly first, then (if it is too slow) profile it and find actual performance bottlenecks, and fix them. Don't corrupt the database... – Marc Gravell Jun 28 '09 at 11:45
  • In particular, your mutex does **nothing** to help when your exe dies because of external factors ("kill process", BSOD, or (more likely) a power failure). Only a transaction is designed to make your insert safe. Why don't you like them? – Marc Gravell Jun 28 '09 at 11:55
  • insertion done every second and it should not take more than second, till now it takes 300 milli seconds, I think if I switched to transaction the performance will decrease, but I'll try – Ahmed Jun 28 '09 at 12:07

3 Answers3

8

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");
    }
}
Community
  • 1
  • 1
Marc Gravell
  • 1,026,079
  • 266
  • 2,566
  • 2,900
  • True, individual updates should be ACID - but the crash here is self inflicted by calling Abort. – Marc Gravell Jun 28 '09 at 09:51
  • 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 Jun 28 '09 at 09:56
  • +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 Jun 28 '09 at 10:01
  • @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 Jun 28 '09 at 10:05
  • 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 Jun 28 '09 at 10:36
  • @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 Jun 28 '09 at 11:00
  • @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 Jun 28 '09 at 11:11
  • @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 Jun 28 '09 at 11:26
  • 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 Jun 28 '09 at 11:26
  • @Sam - I agree fully. Mutex dost not a transaction make. – Marc Gravell Jun 28 '09 at 11:27
  • @sam, I am using mutex for two things one for updating datatables , second thing is to prevent thread.abort during db insertion – Ahmed Jun 28 '09 at 11:42
  • @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 Jun 28 '09 at 12:04
6

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
  • 128,308
  • 78
  • 326
  • 506
  • 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 Jun 28 '09 at 10:58
  • 1
    After the comment and your edit, I wish I could upvote this a second time. – Marc Gravell Jun 28 '09 at 11:42
  • 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 Jun 28 '09 at 12:02
3

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
  • 11,894
  • 12
  • 69
  • 85