¿Existe un patrón de espera mejor para c #?

78

Me he encontrado codificando este tipo de cosas varias veces.

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

Parece un código incorrecto, ¿hay una mejor manera de hacer esto / es un síntoma de un mal diseño?

probablemente en la playa
fuente

Respuestas:

98

Una forma mucho mejor de implementar este patrón es hacer que su Thingobjeto exponga un evento en el que el consumidor puede esperar. Por ejemplo, a ManualResetEvento AutoResetEvent. Esto simplifica enormemente su código de consumidor para que sea el siguiente

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

// It happened

El código en el Thinglateral tampoco es realmente más complejo.

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

  private void TheAction() {
    ...
    // Done.  Signal the listeners
    ManualResetEvent.Set();
  }
}
JaredPar
fuente
+1 gracias Jared, que maneja el caso de excepción de que no sucede muy bien
probablemente en la playa
¡Creo que lo necesitas! la sentencia if, de lo contrario, lanzará una excepción cuando suceda.
SwDevMan81
¿Cuándo usarías cada uno? (Automático
vs.Manual
Auto siempre permitirá que pase exactamente un hilo en espera, ya que se restablece tan pronto como se suelta el primero. Manual permite cualquier hilo que esté esperando hasta que se restablezca manualmente.
ForbesLindesay
@Vinko: agregando al comentario de Tuskan, si desea verificar el estado del ResetEvent después de que se active o se agote el tiempo de espera, entonces tendrá que usar un ManualResetEvent, ya que AutoResetEvent se restablece después de que regresa de la llamada WaitOne. Entonces, el ejemplo que tiene el OP requeriría un ManualResetEvent, donde el ejemplo de JaredPar no verifica el estado y funcionaría mejor con AutoResetEvent
SwDevMan81
29

Utilice eventos.

Haga que lo que está esperando genere un evento cuando haya terminado (o no haya terminado dentro del tiempo asignado) y luego maneje el evento en su aplicación principal.

De esa forma no tendrás ningún Sleepbucle.

ChrisF
fuente
+1 Gracias Chris, ¿cómo se ocupa de los eventos que no suceden dentro de un tiempo determinado (lo que me importa en estos casos)? En mi cabeza todavía estaría durmiendo.
probablemente en la playa
@Richard - Vea la respuesta de
JaredPar
12

Un bucle no es una forma TERRIBLE de esperar algo, si no hay nada más que pueda hacer su programa mientras espera (por ejemplo, mientras se conecta a una base de datos). Sin embargo, veo algunos problemas con el tuyo.

    //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();
    }

En resumen, el código anterior se parece más a un "bucle de reintento", que ha sido bastardado para funcionar más como un tiempo de espera. Así es como estructuraría un ciclo de tiempo de espera:

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
fuente
1
Thread.Yield es interesante, aunque DateTime.Now es más lento que DateTime.UtcNow y startTime.Add (timeout) se evalúa en cada iteración.
Steve Dunn
1
La optimización prematura es la fuente de todos los males. Tiene razón, por supuesto, pero agregar un TimeSpan a un DateTime no es muy costoso, y DateTime.Now solo tiene que compensar las horas. En general, sus optimizaciones no tendrán tanto efecto como deshacerse del sueño.
KeithS
Thread.Yield es un error si no hay hilos de espera listos para ejecutarse
David Heffernan
2
-1: ¡Dios mío! ¡Quememos esa CPU poniéndola en bucle como loca! Mire, al escribir código que se ejecuta en CLR, debería intentar pensar en un nivel superior. ¡Esto no es ensamblaje y no está codificando un PIC!
Bruno Reis
Si sucede algo en segundo plano, Thread.Yield () suspenderá el bucle. Debido a que debería estar sucediendo algo (sea lo que sea que estemos esperando, como mínimo), no quemará la CPU.
KeithS
9

Si es posible, envuelva el procesamiento asincrónico en un archivo Task<T>. Esto proporciona lo mejor de todos los mundos:

  • Puede responder a la finalización de una manera similar a un evento utilizando continuaciones de tareas .
  • Puede esperar utilizando el asa de espera de la terminación porque Task<T>implements IAsyncResult.
  • Las tareas se pueden componer fácilmente usando Async CTP; también juegan bien con Rx.
  • Las tareas tienen un sistema de manejo de excepciones incorporado muy limpio (en particular, conservan correctamente el seguimiento de la pila).

Si necesita utilizar un tiempo de espera, Rx o Async CTP pueden proporcionarlo.

Stephen Cleary
fuente
¿Alguien debería realmente usar Async CTP en producción? Me doy cuenta de que estará cerca del producto final, pero sigue siendo CTP.
VoodooChild
Es tu elección; Ciertamente lo soy. Si lo prefiere, también puede redactar fácilmente Tareas con Rx.
Stephen Cleary
5

Echaría un vistazo a la clase WaitHandle . Específicamente la clase ManualResetEvent que espera hasta que se establece el objeto. También puede especificar valores de tiempo de espera para él y verificar si se configuró posteriormente.

// 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
fuente
2

Una llamada a Thread.Sleepsiempre es una espera activa que debe evitarse.
Una alternativa sería utilizar un temporizador. Para un uso más fácil, podría encapsularlo en una clase.

Daniel Hilgarth
fuente
Thread.Sleep siempre tiene un mal ajuste, lo que me hace preguntarme, ¿cuándo es un buen momento para usar Thread.Sleep?
CheckRaise
2
@CheckRaise: utilícelo cuando desee esperar una cantidad de tiempo definida, y no por una condición.
Bruno Reis
2

Por lo general, desaconsejo lanzar excepciones.

// 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
fuente
@lshpeck: ¿hay alguna razón para no lanzar excepciones aquí? Es algo que esperaría que suceda si el servicio hermano está funcionando
probablemente en la playa
El código real verifica que otro servicio esté realizando un trabajo. Si el servicio no está activado, fallará, por lo que la excepción se lanza y se captura más arriba en la pila.
probablemente en la playa
2

Creo que deberías usar AutoResetEvents. Funcionan muy bien cuando estás esperando que otro hilo termine su tarea

Ejemplo:

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
fuente
2

Así es como puede hacerlo con System.Threading.Tasks:

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

Pero si no puede usar Tasks por algún motivo (como un requisito de .Net 2.0), puede usarlo ManualResetEventcomo se menciona en la respuesta de JaredPar o usar algo como esto:

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);
        }
    }
}

Con el enfoque Wait / Pulse no crea eventos explícitamente, por lo que no necesita preocuparse por eliminarlos.

Ejemplo de uso:

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