2011-09-19 1 views
3

J'ai fait cet échantillon producteur-consommateur, mais je ne sais pas pourquoi il se fige à la fin. Où est le problème? Si je mets un point d'arrêt à la ligne setNum (-99); et après la pause, continuez, ça finit bien. S'il vous plaît dites-moi aussi si ce code est ok et threadsafe. Cela doit fonctionner comme ça, alors que le consommateur traite sa valeur donnée, toutes les autres valeurs du producteur doivent être ignorées. Je suis très nouveau dans le multithreading.Pourquoi ce fil de consommateur-producteur est-il gelé?

class Program 
{ 
    delegate void SetNumberDelegate(int number); 
    static void Main(string[] args) 
    { 
     Random rnd = new Random(); 
     ConsumerClass consumerClass = new ConsumerClass(); 
     SetNumberDelegate setNum = new SetNumberDelegate(consumerClass.setNumber); 
     Thread.Sleep(20); 
     int num; 
     int count = 0; 
     Console.WriteLine("Start"); 
     while (count++ < 100) 
     { 
      num = rnd.Next(0, 100); 
      Console.WriteLine("Generated number {0}", num); 
      if (num > 30) 
      { 
       setNum(num); 
      } 
     } 
     setNum(-99); 
     Console.WriteLine("End"); 
     Console.ReadKey(); 
    } 
} 

class ConsumerClass : IDisposable 
{ 
    private int number; 
    private object locker = new object(); 
    private EventWaitHandle _wh = new AutoResetEvent(false); 
    private Thread _consumerThread; 

    public ConsumerClass() 
    { 
     number = -1; 
     _consumerThread = new Thread(consumeNumbers); 
     _consumerThread.Start(); 
    } 

    public void Dispose() 
    { 
     setNumber(-99); 
     _consumerThread.Join();   
     _wh.Close(); 
    } 

    public void setNumber(int num) 
    { 
     if (Monitor.TryEnter(locker)) 
     { 
      try 
      { 
       number = num; 
       Console.WriteLine("Setting number {0}", number); 
      } 
      finally 
      { 
       // Ensure that the lock is released. 
       Monitor.Exit(locker); 
      } 
      _wh.Set(); 
     } 
    } 

    public void consumeNumbers() 
    { 
     while (true) 
     { 
      Monitor.Enter(locker); 
      if (number > -1) 
      { 
       try 
       { 
        Console.WriteLine("Processing number:{0}", number); 
        // simulate some work with number e.g. computing and storing to db 
        Thread.Sleep(20); 
        Console.WriteLine("Done"); 
        number = -1; 
       } 
       finally 
       { 
        Monitor.Exit(locker); 
       } 
      } 
      else 
      { 
       if (number == -99) 
       { 
        Console.WriteLine("Consumer thread exit"); 
        return; 
       } 
       Monitor.Exit(locker); 
       _wh.WaitOne();   // No more tasks - wait for a signal 
      } 
     } 
    } 
} 
+0

Les questions pour CodeReview doivent présenter des extraits de travail à la recherche de conseils pour le refactoring, l'utilisation de patterns, etc. Si quelque chose ne fonctionne pas, la question doit être envoyée à StackOverflow. – IAbstract

+0

toute personne qui a des privilèges, s'il vous plaît le déplacer vers stackoverflow. Merci. – BreteP

+0

vous pouvez signaler et demander au modérateur de migrer ... – IAbstract

Répondre

2

Rewrite setNumber comme celui-ci pour voir votre problème:

public void setNumber(int num) { 
    if (Monitor.TryEnter(locker)) { 
     // etc.. 
    } 
    else Console.WriteLine("Number {0} will never make it to the consumer", num); 
} 

Vous aurez bloquer, en attendant que le consommateur soit prêt à consommer ou à utiliser une file d'attente.

0

Monitor.TryEnter(locker); sera généralement l'échec (y compris pour les -99) de sorte que vous ne vont pas à définir un bon nombre des valeurs, ce qui est la raison pour laquelle la sortie fait défaut dans les états de réglage. C'est parce qu'il n'attendra pas pour acquérir le verrou qu'il retournera juste faux.

0

Le problème semble être dans la dernière partie du code. Vous tenez le verrou lorsque vous exécutez ceci:

 else 
     { 
      if (number == -99) 
      { 
       Console.WriteLine("Consumer thread exit"); 
       return; 
      } 
      Monitor.Exit(locker); 
      _wh.WaitOne();   // No more tasks - wait for a signal 
     } 

Donc, si number == 99, la méthode retourne sans relâcher le verrou.

Votre méthode ConsumeNumbers est trop complexe. Vous pouvez le simplifier:

while (true) 
{ 
    _wh.WaitOne(); 
    lock (locker) 
    { 
     if (number == -99) 
      break; 
     if (number > -1) 
     { 
      // process the number. 
      number = -1; 
     } 
    } 
} 

Cela fera la même chose, et le code est beaucoup plus simple.

Par ailleurs, la construction:

lock (locker) 
{ 
    // do stuff here 
} 

est le même que:

Monitor.Enter(locker); 
try 
{ 
    // do stuff here 
} 
finally 
{ 
    Monitor.Exit(locker); 
}