I've been doing some code reviews recently, looking in particular for threading issues. It's particularly important in this case because the code in question is a service that needs to run for a long time under high load. It also needs to shut down gracefully if something wonky happens.
One of the things I noticed was some weirdness in the OnStop method of the service, where it iterates through its child threads and tries to get them to shut down gracefully, calling Abort if they won't. Since it turns out that everything important is protected by a database transaction anyway, I suggested a shift to a slightly different model: set it up so it doesn't matter if the threads stop at all.
Part of doing this involves changing the threads to run as background threads. A background thread is represented by a System.Threading.Thread object like any other thread in the CLR, but it has had its IsBackground property set to true. When you do this, you're saying, "It's okay if the process shuts down while this thread is still running." You may have encountered this already if you've ever used the .NET thread pool for anything - all of its threads are background threads, so even if one of them is still doing something, as soon as your last non-background thread goes away, the process dies.
Of course, every effort should be made to ensure that the threads exit cleanly, since proper cleanup is better than whatever automatic cleanup you get when a process dies. But this change should at the very least help the service from hanging on shutdown, which makes it easier to support.
Good Post!
ReplyDeleteOne thing to note here is that you can have multiple shared services per process/AppDomain. Therefore you should definitely try to stop all the threads of the service being stopped. I'm not saying you should try to abort your threads using Thread.Abort (which is always a questionable thing to do), but as you say, every effort should be made to ensure that all threads of the service are stopped cleanly.
ReplyDeleteGood point. In our case we know we only have one service, but that won't always be the case.
ReplyDelete