Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Is this thread safe?

Is this thread safe?

private static bool close_thread_running = false;
public static void StartBrowserCleaning()
{
    lock (close_thread_running)
    {
        if (close_thread_running)
            return;

        close_thread_running = true;
    }

    Thread thread = new Thread(new ThreadStart(delegate()
    {
        while (true)
        {
            lock (close_thread_running)
            {
                if (!close_thread_running)
                    break;
            }

            CleanBrowsers();

            Thread.Sleep(5000);
        }
    }));

    thread.Start();
}

public static void StopBrowserCleaning()
{
    lock (close_thread_running)
    {
        close_thread_running = false;
    }
}
like image 886
Ashley Simpson Avatar asked Dec 08 '22 06:12

Ashley Simpson


1 Answers

Well, it won't even compile, because you're trying to lock on a value type.

Introduce a separate lock variable of a reference type, e.g.

private static readonly object padlock = new object();

Aside from that:

If StopBrowserCleaning() is called while there is a cleaning thread (while it's sleeping), but then StartBrowserCleaning() is called again before the first thread notices that it's meant to shut down, you'll end up with two threads.

You might want to consider having two variables - one for "is there meant to be a cleaning thread" and one for "is there actually a cleaning thread."

Also, if you use a monitor with Wait/Pulse, or an EventHandle (e.g. ManualResetEvent) you can make your sleep a more reactive waiting time, where a request to stop will be handled more quickly.

like image 68
Jon Skeet Avatar answered Dec 31 '22 03:12

Jon Skeet