Moonshot & `Thread.Abort` - dealing with the fallout

Recently, I encountered an anomaly in the manner by which a worker thread was terminated. It prompted me to do some searching and I realized that I was constructing thread code in a haphazard manner. In hopes of beating this subject to death, I'd like some opinions, please.

The issue is real and serious; the use of a Thread.Abort locked up the application and prevented further disposals and terminations of a very formidable (I won't say "dangerous," but...) industrial device. Luckily programmable logic stepped in, but any safety systems anomaly is of great concern.

Consider a simple thread example:

```
using System;
using System.Diagnostics;
using System.Threading;

namespace RapperThreads
{
class Program
{
static void Main(string[] args)
{
Debug.Assert(!rapperEnd.WaitOne(0));
Console.Write("We choose");
rapperRefrain = new string[] { " go to the moon,", " and do the other things," };
for (int index = 0; index < rapperThread.Length; index++)
{
rapperThread[index] = new Thread(JFKRapper);
rapperThread[index].IsBackground = true;
rapperThread[index].Name = rapperRefrain[index];
rapperThread[index].Start((object)index);
}
for (int index = 0; index < 3; index++)
{
Console.Write(" in this decade to");
Thread.Sleep(900);
}
rapperEnd.Set();
Thread.Sleep(0);
Console.WriteLine(" not because they are easy, but because they are hard...");
lock (rapperEnd)
foreach (Thread thread in rapperThread)
if (thread != null)
if (thread.IsAlive)
{
try { thread.Abort(); }
catch (Exception ex) { Console.WriteLine(ex.ToString()); }
try { if (!thread.J

I think you definitely have a deadlock here with your use of rapperEnd. The main thread holds a lock on it while waiting for the worker threads to complete, and the worker threads need to acquire the lock in order to end. The problem is that the main thread holds the lock much longer than it needs to. In addition, rapperEnd has two totally unrelated responsibilities: as an Event and as a Monitor, which I would split into different objects.


Assuming that the lock is a good idea, must the lock object (in this case, the ManaualResetEvent) be disposed of explicitly (it isn't)?

You don't have to clean up objects you lock, but you should clean up events.

To address some other questions:


we watched that call lock up the application because the (equivalent)
to the rapperEnd.Set statement took too long to complete (many
subscribers to an event chain)

That's not how events work, setting an event should be nearly instantaneous. The Events classes on Windows are just thin wrappers around native Win32 event objects.


See the Thread.Sleep(0) just after rapperEnd.Set? Does that actually give the worker
thread any cycles?

Sleep(0) tells the scheduler that the current thread doesn't need the remainder of its time slice. I wouldn't describe it as giving anything to another thread, it's all up to the scheduler to give processor time to threads.

StackExchange Code Review Q#54163, answer score: 2