2010-10-25 26 views
3

Dans mon service Windows, je crée un thread de premier plan "parent" qui génère à son tour des threads "enfants" en utilisant ThreadPool (ce qui signifie qu'ils sont en arrière-plan) pour exécuter des tâches.Fermer le thread de premier plan gracieusement sur Windows service stop

Quelle est la meilleure façon de fermer le fil d'avant-plan gracieusement sur l'arrêt de service des fenêtres?

Voici mon implémentation actuelle (dépouillé de la logique spécifique à la tâche):

public partial class TaskScheduler : ServiceBase 
{ 
    private static AutoResetEvent _finishedTaskAutoResetEvent = new AutoResetEvent(false); 

    //This flag is used to increase chances of the Spawning Thread to finish gracefully when service stops. 
    private bool StopRequested { get; set; } 

    private int _executingTasksCount; 

    private int ExecutingTasksCount { get { return _executingTasksCount; } } 

    private void IncCurrentTasksCount() 
    { 
     Interlocked.Increment(ref _executingTasksCount); 
    } 

    private void DecCurrentTasksCount() 
    { 
     Interlocked.Decrement(ref _executingTasksCount); 
    } 

    public TaskScheduler() 
    { 
     InitializeComponent(); 

     Thread spawningThread = new Thread(DoSpawnTaskExecutionThreads); 

     spawningThread.Name = "Spawning Thread"; 
     spawningThread.IsBackground = false; 
     spawningThread.Start(); 
    } 

    protected override void OnStart(string[] args) 
    { 
    } 

    protected override void OnStop() 
    { 
     StopRequested = true; 
    } 

    private void DoSpawnTaskExecutionThreads() 
    { 
     //We check StopRequested to try and finish this thread gracefully when service stops. 
     while (!StopRequested) 
     { 
      while (!StopRequested && ExecutingTasksCount < MaxPooledTasks) 
      { 
       ThreadPool.QueueUserWorkItem(ExecuteTask, new Task()); 

       IncCurrentTasksCount(); 
      } 

      _finishedTaskAutoResetEvent.WaitOne(); 
     } 

     //Either all task execution threads will finish or the process will be terminated forcibly. 
     while (ExecutingTasksCount > 0) 
     { 
      Thread.Sleep(200); //Check five times a second. 
     } 

     _eventLog.WriteEntry("The Spawning Thread finished along with task execution threads."); 
    } 

    private void ExecuteTask(object state) 
    { 
     try 
     { 
      Task task = (Task)state; 

      task.Execute(); 
     } 
     catch 
     { 
      // Handle exception. 
     } 
     finally 
     { 
      DecCurrentTasksCount(); 
      _finishedTaskAutoResetEvent.Set(); 
     } 
    } 

}

Répondre

3

Je vois un couple de problèmes avec le code.

  • La vérification de StopRequested n'est pas adaptée aux threads.
  • La vérification de ExecutingTaskCount n'est pas adaptée aux threads.
  • Depuis _finishedTaskAutoResetEvent est un AutoResetEvent signaux peuvent se perdre parce que WaitHandle ne maintient pas un nombre. Peut-être que c'est ce que vous voulez, mais cela pourrait entraîner une rotation étrange des boucles imbriquées while.

Voici comment je refactoriserais votre code. Il utilise la classe CountdownEvent qui est disponible dans .NET 4.0.

public class TaskScheduler : ServiceBase 
{ 
    private m_Stop as ManualResetEvent = new ManualResetEvent(false); 

    protected override void OnStart(string[] args)   
    {   
     var thread = new Thread(DoSpawnTaskExecutionThreads); 
     thread.Name = "Spawning Thread"; 
     thread.IsBackground = false; 
     thread.Start(); 
    }   

    protected override OnStop() 
    { 
     m_Stop.Set(); 
    } 

    public DoSpawnTaskExecutionThreads() 
    { 
     // The semaphore will control how many concurrent tasks can run. 
     var pool = new Semaphore(MaxPooledThreads, MaxPooledThreads); 

     // The countdown event will be used to wait for any pending tasks. 
     // Initialize the count to 1 so that we treat this thread as if it 
     // were a work item. This is necessary to avoid a subtle race 
     // with a real work item that completes quickly. 
     var tasks = new CountdownEvent(1); 

     // This array will be used to control the spinning of the loop. 
     var all = new WaitHandle[] { pool, m_Stop }; 

     while (WaitHandle.WaitAny(all) == 0) 
     { 
     // Indicate that there is another task. 
     tasks.AddCount(); 

     // Queue the task. 
     Thread.QueueUserWorkItem(
      (state) => 
      { 
      try 
      { 
       var task = (Task)state; 
       task.Execute(); 
      } 
      finally 
      { 
       pool.Release(); // Allow another task to be queued. 
       tasks.Signal(); // Indicate that this task is complete. 
      } 
      }, new Task()); 
     } 

     // Indicate that the main thread is complete. 
     tasks.Signal(); 

     // Wait for all pending tasks. 
     tasks.Wait(); 
    } 
} 
+0

Merci pour une explication si complète et un exemple de code. J'ai quelques questions cependant: 1) "La vérification de ExecutingTaskCount n'est pas thread-safe": Pourquoi? Je ne fais que le modifier en utilisant la classe Interlocked. Si je voulais toujours l'utiliser pour une raison quelconque, comment pourrais-je y aller? Quand recommanderiez-vous d'utiliser la classe Interlocked? 2) "... _finishedTaskAutoResetEvent est un AutoResetEvent les signaux peuvent être perdus parce que WaitHandle ne maintient pas un compte ...": Quelle pourrait être la cause d'une telle situation? Tâche de lancer une exception non gérée et de ne pas la gérer pour une raison quelconque? – Den

+0

RE # 1 ... Pour rendre thread-safe 'ExecutingTasksCount', vous devrez faire une lecture volatile de '_executingTasksCount'. Cela peut être fait en utilisant la méthode 'Interlocked.CompareExchange' ou en marquant la variable comme' volatile'. –

+0

RE # 2 ... Imaginez un scénario hypothétique (très improbable) où tous les threads de tâche sont préemptés entre 'DecCurrentTasksCount' et' _finishedTaskAutoResetEvent.Set'. Je pense que vos boucles imbriquées protègent contre tout problème, mais je vois des façons bizarres de se comporter. Encore une fois, je ne pense pas qu'il y ait vraiment de problème avec cette approche, mais il est difficile d'y penser. –

2

Il y a un problèmes que je vois ici:

StopRequested ne doit pas être une propriété automatique . Vous devez définir ceci comme une propriété avec un champ de support, afin de le marquer volatile.

private volatile bool stopRequested; 
private bool StopRequested 
{ 
    get { return this.stopRequested; } 
    set { this.stopRequested = value; } 
} 

Sans cela, il est possible que la condition de sortie ne peut pas être vu (au moins tout de suite) par votre fil quand il est fixé par le service.

En outre, si .NET 4 est une option, il est beaucoup plus simple de concevoir ce qui pourrait être fait en utilisant CancellationToken et BlockingCollection<T>.

+0

Le seul endroit où j'ai l'intention de modifier StopRequested dans est OnStop(). Merci pour votre suggestion – Den

+2

@Den: OnStop sera appelé à partir d'un thread séparé. Sans cela, le thread de TaskScheduler ne verra pas (nécessairement) le changement. –

+1

@Den: Ce qui se passerait, c'est que JIT pourrait voir que 'DoSpawnTaskExecutionThreads' ne change jamais' StopRequested' et qu'il pourrait donc essayer d'optimiser les lectures répétitives en les soulevant et en les combinant en dehors et au dessus des boucles. L'appel 'WaitOne' que vous avez dans la boucle arrêtera le compilateur de faire cette optimisation dans la réalité, mais c'est le cas de travailler par accident. Il est préférable de suivre les conseils de Reed ici pour qu'il n'y ait aucun doute. –

0

Vous pouvez utiliser la méthode Join pour "fermer" le thread. MSDN a quelques informations sur la méthode.

+1

Cela ne fonctionnera pas ici - vous ne pouvez pas bloquer le thread de service, ou l'hôte de service le supprimera de force, car il y a un délai d'expiration pour arrêter les services. –