Gibt es ein besseres Wartemuster für c #?

78

Ich habe einige Male solche Dinge programmiert.

for (int i = 0; i < 10; i++)
{
   if (Thing.WaitingFor())
   {
      break;
   }
   Thread.Sleep(sleep_time);
}
if(!Thing.WaitingFor())
{
   throw new ItDidntHappenException();
}

Es sieht einfach nach schlechtem Code aus. Gibt es einen besseren Weg, dies zu tun? Ist es ein Symptom für schlechtes Design?

wahrscheinlich am Strand
quelle

Antworten:

98

Eine viel bessere Möglichkeit, dieses Muster zu implementieren, besteht darin, Ihr ThingObjekt ein Ereignis anzeigen zu lassen, auf das der Verbraucher warten kann. Zum Beispiel ein ManualResetEventoder AutoResetEvent. Dies vereinfacht Ihren Verbrauchercode erheblich und sieht wie folgt aus

if (!Thing.ManualResetEvent.WaitOne(sleep_time)) {
  throw new ItDidntHappen();
}

// It happened

Der Code auf der ThingSeite ist auch nicht wirklich komplexer.

public sealed class Thing {
  public readonly ManualResetEvent ManualResetEvent = new ManualResetEvent(false);

  private void TheAction() {
    ...
    // Done.  Signal the listeners
    ManualResetEvent.Set();
  }
}
JaredPar
quelle
+1 danke Jared - das behandelt den Ausnahmefall, dass es nicht sehr ordentlich passiert
wahrscheinlich am Strand
Ich denke du musst! die if-Anweisung, andernfalls wird eine Ausnahme ausgelöst, wenn dies geschieht.
SwDevMan81
Wann würden Sie jeden verwenden? (Auto vs. Manuell)
Vinko Vrsalovic
Auto lässt immer genau einen wartenden Thread durch, da er zurückgesetzt wird, sobald der erste freigegeben wird. Manuell erlaubt alle Threads, die warten, bis sie manuell zurückgesetzt werden.
ForbesLindesay
@Vinko - Wenn Sie zusätzlich zu Tuskans Kommentar den Status des ResetEvent nach dessen Auslösung oder Zeitüberschreitung überprüfen möchten, müssen Sie ein ManualResetEvent verwenden, da das AutoResetEvent nach der Rückkehr vom WaitOne-Aufruf zurückgesetzt wird. Für das Beispiel des OP wäre also ein ManualResetEvent erforderlich, bei dem das Beispiel von JaredPar den Status nicht überprüft und besser mit dem AutoResetEvent
SwDevMan81 vom
29

Verwenden Sie Ereignisse.

Lassen Sie das Ereignis, auf das Sie warten, ein Ereignis auslösen, wenn es beendet ist (oder nicht innerhalb der vorgegebenen Zeit beendet wurde), und behandeln Sie das Ereignis dann in Ihrer Hauptanwendung.

Auf diese Weise haben Sie keine SleepSchleifen.

ChrisF
quelle
+1 Danke Chris, wie können Sie für Ereignisse sorgen, die nicht innerhalb einer bestimmten Zeit stattfinden (was mir in diesen Fällen wichtig ist)? In meinem Kopf würde ich immer noch schlafen.
wahrscheinlich am Strand
@ Richard - Siehe JaredPars Antwort
ChrisF
12

Eine Schleife ist keine SCHRECKLICHE Möglichkeit, auf etwas zu warten, wenn Ihr Programm während des Wartens nichts anderes tun kann (z. B. während der Verbindung zu einer Datenbank). Ich sehe jedoch einige Probleme mit Ihren.

    //It's not apparent why you wait exactly 10 times for this thing to happen
    for (int i = 0; i < 10; i++)
    {
        //A method, to me, indicates significant code behind the scenes.
        //Could this be a property instead, or maybe a shared reference?
        if (Thing.WaitingFor()) 
        {
            break;
        }
        //Sleeping wastes time; the operation could finish halfway through your sleep. 
        //Unless you need the program to pause for exactly a certain time, consider
        //Thread.Yield().
        //Also, adjusting the timeout requires considering how many times you'll loop.
        Thread.Sleep(sleep_time);
    }
    if(!Thing.WaitingFor())
    {
        throw new ItDidntHappenException();
    }

Kurz gesagt, der obige Code sieht eher wie eine "Wiederholungsschleife" aus, die bastardisiert wurde, um eher wie eine Zeitüberschreitung zu funktionieren. So würde ich eine Timeout-Schleife strukturieren:

var complete = false;
var startTime = DateTime.Now;
var timeout = new TimeSpan(0,0,30); //a thirty-second timeout.

//We'll loop as many times as we have to; how we exit this loop is dependent only
//on whether it finished within 30 seconds or not.
while(!complete && DateTime.Now < startTime.Add(timeout))
{
   //A property indicating status; properties should be simpler in function than methods.
   //this one could even be a field.
   if(Thing.WereWaitingOnIsComplete)
   {
      complete = true;
      break;
   }

   //Signals the OS to suspend this thread and run any others that require CPU time.
   //the OS controls when we return, which will likely be far sooner than your Sleep().
   Thread.Yield();
}
//Reduce dependence on Thing using our local.
if(!complete) throw new TimeoutException();
KeithS
quelle
1
Thread.Yield ist interessant, obwohl DateTime.Now langsamer als DateTime.UtcNow ist und startTime.Add (Timeout) bei jeder Iteration ausgewertet wird.
Steve Dunn
1
Vorzeitige Optimierung ist die Wurzel allen Übels. Sie haben natürlich Recht, aber das Hinzufügen eines TimeSpan zu einer DateTime ist nicht sehr teuer, und DateTime.Now muss nur die Stunden ausgleichen. Insgesamt wirken sich Ihre Optimierungen nicht so stark aus, als würden Sie den Schlaf loswerden.
KeithS
Thread.Yield ist ein Noop, wenn keine wartenden Threads zum Ausführen bereit sind
David Heffernan
2
-1: Meine Güte! Brennen wir diese CPU, indem wir sie wie verrückt in eine Schleife setzen! Wenn Sie Code schreiben, der auf der CLR ausgeführt wird, sollten Sie wirklich versuchen, auf einer höheren Ebene zu denken. Dies ist keine Assembly, und Sie codieren keinen PIC!
Bruno Reis
Wenn im Hintergrund etwas passiert, unterbricht Thread.Yield () die Schleife. Da etwas passieren sollte (worauf wir zumindest SEHR warten), wird die CPU nicht verbrannt.
KeithS
9

Wenn möglich, lassen Sie die asynchrone Verarbeitung in a einschließen Task<T>. Dies bietet das Beste von allen Welten:

  • Sie können auf die Fertigstellung ereignisartig reagieren, indem Sie Aufgabenfortsetzungen verwenden .
  • Sie können mit dem wartbaren Handle der Vervollständigung warten, da Task<T>implementiert IAsyncResult.
  • Aufgaben sind leicht zusammensetzbar mit dem Async CTP; Sie spielen auch gut mit Rx.
  • Aufgaben verfügen über ein sehr sauberes integriertes Ausnahmebehandlungssystem (insbesondere wird die Stapelverfolgung korrekt beibehalten).

Wenn Sie ein Timeout verwenden müssen, kann Rx oder das Async CTP dies bereitstellen.

Stephen Cleary
quelle
Sollte jemand das Async CTP wirklich in der Produktion verwenden? Mir ist klar, dass es dem Endprodukt nahe kommen wird, aber es ist immer noch CTP.
VoodooChild
Es ist Ihre Wahl; Ich bin es auf jeden Fall. Wenn Sie möchten, können Sie Aufgaben auch problemlos mit Rx erstellen.
Stephen Cleary
5

Ich würde mir die WaitHandle- Klasse ansehen . Insbesondere die ManualResetEvent- Klasse, die wartet, bis das Objekt festgelegt ist. Sie können auch Timeout-Werte dafür angeben und prüfen, ob diese anschließend festgelegt wurden.

// Member variable
ManualResetEvent manual = new ManualResetEvent(false); // Not set

// Where you want to wait.
manual.WaitOne(); // Wait for manual.Set() to be called to continue here
if(!manual.WaitOne(0)) // Check if set
{
   throw new ItDidntHappenException();
}
SwDevMan81
quelle
2

Ein Anruf bei Thread.SleepAlways ist ein aktives Warten, das vermieden werden sollte.
Eine Alternative wäre die Verwendung eines Timers. Zur einfacheren Verwendung können Sie dies in eine Klasse einkapseln.

Daniel Hilgarth
quelle
Thread.Sleep bekommt immer einen schlechten Wrap, was mich wundern lässt, wann ein wirklich guter Zeitpunkt ist, um Thread.Sleep zu verwenden?
CheckRaise
2
@CheckRaise: Verwenden Sie diese Option, wenn Sie auf eine definierte Zeit und nicht auf eine Bedingung warten möchten.
Bruno Reis
2

Ich rate normalerweise davon ab, Ausnahmen zu werfen.

// Inside a method...
checks=0;
while(!Thing.WaitingFor() && ++checks<10) {
    Thread.Sleep(sleep_time);
}
return checks<10; //False = We didn't find it, true = we did
Ishpeck
quelle
@lshpeck - gibt es einen Grund, hier keine Ausnahmen zu werfen? Es ist etwas, was ich erwarten würde, wenn der Schwesterservice läuft
wahrscheinlich am Strand
Der eigentliche Code überprüft, ob ein anderer Dienst eine Arbeit ausführt. Wenn der Dienst nicht aktiviert ist, schlägt er fehl, sodass die Ausnahme ausgelöst und weiter oben im Stapel abgefangen wird.
wahrscheinlich am Strand
2

Ich denke, Sie sollten AutoResetEvents verwenden. Sie funktionieren hervorragend, wenn Sie darauf warten, dass ein anderer Thread seine Aufgabe beendet

Beispiel:

AutoResetEvent hasItem;
AutoResetEvent doneWithItem;
int jobitem;

public void ThreadOne()
{
 int i;
 while(true)
  {
  //SomeLongJob
  i++;
  jobitem = i;
  hasItem.Set();
  doneWithItem.WaitOne();
  }
}

public void ThreadTwo()
{
 while(true)
 {
  hasItem.WaitOne();
  ProcessItem(jobitem);
  doneWithItem.Set();

 }
}
Kornél Regius
quelle
2

So können Sie es machen System.Threading.Tasks:

Task t = Task.Factory.StartNew(
    () =>
    {
        Thread.Sleep(1000);
    });
if (t.Wait(500))
{
    Console.WriteLine("Success.");
}
else
{
    Console.WriteLine("Timeout.");
}

Wenn Sie Tasks jedoch aus irgendeinem Grund nicht verwenden können (z. ManualResetEventB. gemäß einer Anforderung von .Net 2.0), können Sie diese wie in der Antwort von JaredPar erwähnt verwenden oder Folgendes verwenden:

public class RunHelper
{
    private readonly object _gate = new object();
    private bool _finished;
    public RunHelper(Action action)
    {
        ThreadPool.QueueUserWorkItem(
            s =>
            {
                action();
                lock (_gate)
                {
                    _finished = true;
                    Monitor.Pulse(_gate);
                }
            });
    }

    public bool Wait(int milliseconds)
    {
        lock (_gate)
        {
            if (_finished)
            {
                return true;
            }

            return Monitor.Wait(_gate, milliseconds);
        }
    }
}

Mit dem Wait / Pulse-Ansatz erstellen Sie keine expliziten Ereignisse, sodass Sie sich nicht um deren Entsorgung kümmern müssen.

Anwendungsbeispiel:

var rh = new RunHelper(
    () =>
    {
        Thread.Sleep(1000);
    });
if (rh.Wait(500))
{
    Console.WriteLine("Success.");
}
else
{
    Console.WriteLine("Timeout.");
}
Gebb
quelle