1

The Problem

I've created a thread in c# (on windows 10), which asks a server for some data:

  • Such a thread should be created each time the user enters a certain menu (mainMenuJoinRoomButtonClick())
  • It should be closed each time the users exits that menu (we exit with roomListBackButtonClick())

This is done in order for us to save resources and prevent errors which occur if the thread isn't closed. (please see the code below)

I tried using threadName.Abort() but it gives me the following error:

System.PlatformNotSupportedException: 'Thread abort is not supported on this platform.'

I also tried threadName.Interrupt() which produces this error (the error occurs in the line: Thread.Sleep(3000);):

System.Threading.ThreadInterruptedException: 'Thread was interrupted from a waiting state.'

What is the problem here and how can I close the thread?


Research Done

Looking around the internet has showed there is no good way to abort a thread, is that so?

I also found this answer (first one) which uses Thread.sleep() but this doesn't seem like a real solution because it doesn't release the thread's resources, and I'm unsure how to implement this solution in my code.


The Code

namespace TriviaClient
{
    public partial class MainWindow : Window
    {
        public const int GET_ROOMS_REQUEST = 92;

        public Thread refreshRoomsListThread;

        public static NetworkStream clientStream;

        public MainWindow()
        {
            StartClient();
            InitializeComponent();
        }

        public void refreshRoomList()
        {
            while (true)
            {
                this.Dispatcher.Invoke((Action)(() =>
                {
                    roomsListErrorBox.Text = "";
                }));
                serializeAndSendMessage(GET_ROOMS_REQUEST, "");
                Dictionary<string, object> response = receiveAndDeserializeMessage();

                if (response.ContainsKey("status") && (string)response["status"] == "1")
                {
                    this.Dispatcher.Invoke((Action)(() =>
                    {
                        roomsList.Items.Clear();
                    }));

                    Dictionary<int, string> rooms = new Dictionary<int, string>();

                    JArray Jrooms = (JArray)response["rooms"];


                    foreach (JArray room in Jrooms)
                    {
                        rooms.Add((int)room[0], (string)room[1]);
                    }

                    foreach (KeyValuePair<int, string> roomInDict in rooms)
                    {
                        this.Dispatcher.Invoke((Action)(() =>
                        {
                            roomsList.Items.Add(new Label().Content = $"{roomInDict.Key}: {roomInDict.Value}");
                        }));
                    }

                }
                else if (response.ContainsKey("message"))
                {
                    this.Dispatcher.Invoke((Action)(() =>
                    {
                        roomsListErrorBox.Text = (string)response["message"];
                    }));
                }
                Thread.Sleep(3000);
            }
        }
        private void roomListBackButtonClick(object sender, RoutedEventArgs e)
        {
            refreshRoomsListThread.Abort();

        }
        private void mainMenuJoinRoomButtonClick(object sender, RoutedEventArgs e)
        {
            roomsListErrorBox.Text = "";

            refreshRoomsListThread = new Thread(refreshRoomList);
            refreshRoomsListThread.Start();

        }
    }
}

snatchysquid
  • 1,283
  • 9
  • 24
  • 1
    `while (true) { ... Thread.Sleep(3000); }` - why aren't you using a timer that can be stopped at any time? Preferrably a DispatcherTimer, which would additionally also remove the need for Dispatcher.Invoke. – Clemens Jun 03 '21 at 18:11
  • 4
    What about a Task + CancellationToken? -- Do you have an async version of `serializeAndSendMessage` and `receiveAndDeserializeMessage()` or is it possible to convert them to async (it looks like I/O + JSON serialization)? You're invoking so often that probably just async/await could be used here. – Jimi Jun 03 '21 at 18:11
  • 1
    `Thread.Abort` is never a good idea - particularly if you care about errors and lost resources. Why not use the thread pool, and pass your background work something that says "Ok, quit now" when you no longer need the thread (as suggested by @mm8). You might also want to read up on `async` and `await`. There's probably a better way of doing what you want to do by using more modern constructs. – Flydog57 Jun 03 '21 at 18:34
  • Does the `serializeAndSendMessage` method accept a `CancellationToken` as argument? – Theodor Zoulias Jun 03 '21 at 18:51

3 Answers3

2

You should be using a DispatcherTimer with an async Tick event handler that runs the long-running operation in a Task.

The Tick event handler is executed in the UI thread, so you don't need to use the Dispatcher.

private readonly DispatcherTimer timer = new DispatcherTimer
{
    Interval = TimeSpan.FromSeconds(3)
};

public MainWindow()
{
    StartClient();
    InitializeComponent();

    timer.Tick += OnTimerTick;
}

private void mainMenuJoinRoomButtonClick(object sender, RoutedEventArgs e)
{
    timer.Start();
}

private void roomListBackButtonClick(object sender, RoutedEventArgs e)
{
    timer.Stop();
}

private async void OnTimerTick(object sender, EventArgs e)
{
    roomsListErrorBox.Text = "";

    Dictionary<string, object> response = await Task.Run(
        () =>
        {
            serializeAndSendMessage(GET_ROOMS_REQUEST, "");

            return receiveAndDeserializeMessage();
        });


    if (response.ContainsKey("status") && (string)response["status"] == "1")
    {
        roomsList.Items.Clear();
        // ...
    }
}
Clemens
  • 123,504
  • 12
  • 155
  • 268
-1

Instead of tryng to forcefully abort the thread, you could set a flag to eventually exit the while loop:

private bool _run = true;
public void refreshRoomList()
{
    while (_run)
    {
        ...
    }
}
private void roomListBackButtonClick(object sender, RoutedEventArgs e)
{
    _run = false;
}
mm8
  • 163,881
  • 10
  • 57
  • 88
-4

To resolve this problem you can use a Boolean and let the thread alive here is an example :

        public bool KeepAlive { get; set; } = true;
    private void YourThread()
    {


        while (KeepAlive)
        {
            //Do your task here
        }
    }

when you want to stop the thread set KeepAlive = false;

YacineSM
  • 81
  • 3