views:

158

answers:

1
    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;
  }
 }
+11  A: 

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.

Jon Skeet
I'd say that something which doesn't compile is technically threadsafe - it'll never deadlock or encounter race conditions :-)
Vinko Vrsalovic
does that mean my car is air-safe since it can never fly and thus never encounter a mid-air collision?
icelava
I've never heard about 'air-safety' before, but of course. Why wouldn't it be? It is also space travel safe.
Vinko Vrsalovic
Changed first word from "no" to "well"... I think I'd say that the thread safety of code which doesn't compile is undefined. The code can't run safely on multiple threads, after all.
Jon Skeet