I'm writing a program which updates a display using three threads, two that both write images to the display form, and one to switch between the two when conditions are necessary. I'll include code for all three at the bottom. I've noticed that when the absence of audio prompts for the abortion of the talkingThread, and the creation & start of the neutralThread, the talking images will hang there for a few seconds, the duration varying. I suspect it has something to do with the Sleep calls at the end of the thread, that calling Abort doesn't actually dispose the thread until that sleep is cleared, but I don't know if that would affect another thread's ability to write over the display. Is there any way to ensure that the thread is disposed sooner?

Neutral Loop -

private void neutralLoop() //loop for switching to and maintaining the neutral images
        {  
            while (true)
            {
                if (isBlinking) //if blinking is enabled, check whether or not to blink, and hold for 1 second if so
                {
                    int blinkChance = rnd.Next(2);
                    if (blinkChance == 1)
                    {
                        myImage = this.neutralEyesClosedMap;
                        pictureBox1.Image = myImage;
                        Thread.Sleep(1000);
                    }
                }
                myImage = this.neutralEyesOpenMap; //replace neutral eyes open state, hold for 5 secs
                pictureBox1.Image = myImage;
                Thread.Sleep(5000);
            }
        }

Talking Loop -

private void talkingLoop() //loop for switching to and maintaining the talking images
        {
            while (true)
            {
                if (isBlinking) //if blinking is enabled, check whether or not to blink, and hold for 1 second if so
                {
                    int blinkChance = rnd.Next(2);
                    if (blinkChance == 1)
                    {
                        myImage = this.talkingEyesClosedMap;
                        pictureBox1.Image = myImage;
                        Thread.Sleep(1000);
                    }
                }
                myImage = this.talkingEyesOpenMap; //replace neutral eyes open state, hold for 5 secs
                pictureBox1.Image = myImage;
                Thread.Sleep(5000);
            }
        }

Switch Loop

private void switchLoop()
        {
            neutralThread.Start();
            while (true)
            {
               if (deviceMeter.MasterPeakValue > 0 && deviceMeter.MasterPeakValue < 1) //check for audio
               {
                    if (talkingThread.ThreadState != ThreadState.Running && talkingThread.ThreadState != ThreadState.WaitSleepJoin) //if talkingThread isnt running (neutralThread is running), get rid of it and start talkingThread.
                    {
                        neutralThread.Abort();
                        talkingThread = new Thread(new ThreadStart(talkingLoop));
                        talkingThread.Start();
                    }
               } else {
                    if (neutralThread.ThreadState != ThreadState.Running && neutralThread.ThreadState != ThreadState.WaitSleepJoin) //if neutralThread isnt running (talkingThread is running), get rid of it and start neutralThread.
                    {
                        talkingThread.Abort();
                        neutralThread = new Thread(new ThreadStart(neutralLoop));
                        neutralThread.Start();
                    }
               }
            }
        }

CodePudding user response：

Using Thread.Abort() is a terrible idea that can lead to all kinds of issues. The example also looks like it is modifying UI elements on background threads, and this is also not allowed, and can lead to various issues.

The correct way to cancel some running background task is to do so cooperatively. However, it does not look like we need to use any background threads at all. What we do need is a state-machine and a timer. The state-machine keeps track of what state we are in, what images to use, and what should happen when we switch between states. The timer is used to trigger a state-change after some time.

It is possible to use async/await for this since this will be compiled to a statemachine. while Task.Delay wraps a timer in a way that can be used with async/await. So as far as I can tell you could rewrite your example like this:

        private async Task Loop() 
        {
            while (true)
            {
                cts = new CancellationTokenSource(); // replace the cancellationToken source each iteration
                try
                {
                    if (isBlinking) {
                        int blinkChance = rnd.Next(2);
                        if (blinkChance == 1)
                        {
                            pictureBox1.Image = isneutral ? neutralEyesClosedMap : talkingEyesClosedMap;
                            await Task.Delay(1000, cts.Token);
                        }
                    } 
                    pictureBox1.Image = isneutral ? neutralEyesOpenMap : talkingEyesOpenMap;
                    await Task.Delay(5000, cts.Token);
                }
                // Task.Delay may throw when isneutral changes state, this is expected, just continue
                catch(OperationCanceledException){} 
            }
        }
        public async Task SetMasterPeakValue(double value)
        {
            var oldValue = isneutral;
            isneutral = value > 0 && value < 1;
            if (oldValue != isneutral)
            {
                cts.Cancel(); // cancel the current loop
            }
        }

This should as far as I can tell maintain the same behavior, while running all the code on the main-thread, and avoids all nastiness with threads. Note that this requires the class to be informed when the talking/neutral condition actually changes, my example uses a method, but it could just as well be an event. You probably also need some way to stop the entire loop, this could be something simple as a boolean, or a another cancellationTokenSource.