C
C#•4mo ago
Nacho Man Randy Cabbage

Replacing thread.Abort()

I'm looking at some very old code from .net 3.5 which has a bunch of .Abort() calls like this: 
if (someThread.IsAlive == true)
{
    someThread.Abort();
}

someThread = new Thread(() => SomeFunction(cancellationToken));
someThread.IsBackground = ture;
if (someThread.IsAlive == true)
{
    someThread.Abort();
}

someThread = new Thread(() => SomeFunction(cancellationToken));
someThread.IsBackground = ture;
Would it make sense to just switch these to tasks or is there a different replacement I can make? The most likely scenario is that these run for a long time (possibly days) if that matters.
7 Replies
jcotton42
jcotton42•4mo ago
CancellationToken Have the background work poll it. Task vs thread is irrelevant here.
Nacho Man Randy Cabbage
Nacho Man Randy CabbageOP•4mo ago
hmm, well the background work is already checking if the token is cancelled. I'm guessing the part I posted was setup to "restart" the thread as well as initialize it for the first time. For example there is a Stop() function that simply calls .Cancel() on the token. Here's full example: 
public void Start()
{
    _done = false;
    _someThreadCancelSource = new CancellationTokenSource();
    _someThreadCancelToken = _someThreadCancelSource.Token;

    _someThread = new Thread(() => HeartbeatThread(_someThreadCancelToken));
    _someThread.IsBackground = true;
    _someThread.Name = Name + " Heartbeat";
    try
    {
        _someThread.Start();
    }
    catch
    {
        _someThreadCancelSource.Cancel();
    }
}

private void HeartbeatThread(CancellationToken token)
{
    Thread.Sleep(1000);

    while (!_done && !token.IsCancellationRequested)
    {
        // doing some work

        Thread.Sleep(200);
    }
}

public void Stop()
{
    _done = true;

    if (_someThread != null)
    {
        if (_someThread.IsAlive)
        {
            _someThreadCancelSource.Cancel();
        }
    }
}
public void Start()
{
    _done = false;
    _someThreadCancelSource = new CancellationTokenSource();
    _someThreadCancelToken = _someThreadCancelSource.Token;

    _someThread = new Thread(() => HeartbeatThread(_someThreadCancelToken));
    _someThread.IsBackground = true;
    _someThread.Name = Name + " Heartbeat";
    try
    {
        _someThread.Start();
    }
    catch
    {
        _someThreadCancelSource.Cancel();
    }
}

private void HeartbeatThread(CancellationToken token)
{
    Thread.Sleep(1000);

    while (!_done && !token.IsCancellationRequested)
    {
        // doing some work

        Thread.Sleep(200);
    }
}

public void Stop()
{
    _done = true;

    if (_someThread != null)
    {
        if (_someThread.IsAlive)
        {
            _someThreadCancelSource.Cancel();
        }
    }
}
So instead of the Abort() could I just do a .Join() once the token has been Canceled?
jcotton42
jcotton42•4mo ago
If you want to make sure the thread has stopped, yeah.
wasabi
wasabi•4mo ago
I'd ask why the previous people decided to use Thread.Abort. It is often an attempt to stop something which could not otherwise be stopped.
Petris
Petris•4mo ago
an example of how Abort is bad: if you use it on a thread that's in process.WaitForExit(), the whole process will crash
Nacho Man Randy Cabbage
Nacho Man Randy CabbageOP•4mo ago
Yeah the original creators aren't at the same company anymore. I am assuming it was their way of making sure the thread was stopped before assigning it a new one and starting it up, that's what it reads to me at least Really all it's doing is listening to some udp ports on these threads and receiving/sending data on them
jcotton42
jcotton42•4mo ago
A prime candidate for moving to tasks too then.

Did you find this page helpful?