Questo è thread-safe giusto?

Question

Basta controllare ... _count si accede in sicurezza, giusto?

Entrambi i metodi sono accessibili da più thread.

private int _count; 

public void CheckForWork() { 
    if (_count >= MAXIMUM) return; 
    Interlocked.Increment(ref _count); 
    Task t = Task.Run(() => Work()); 
    t.ContinueWith(CompletedWorkHandler); 
} 

public void CompletedWorkHandler(Task completedTask) { 
    Interlocked.Decrement(ref _count); 
    // Handle errors, etc... 
} 

Answer 1

No, if (_count >= MAXIMUM) return; non è sicuro.

edit: Avresti per chiudere intorno anche la lettura, che dovrebbe quindi logicamente essere raggruppato con l'incremento, quindi mi piacerebbe riscrivere come

private int _count; 

private readonly Object _locker_ = new Object(); 

public void CheckForWork() { 
    lock(_locker_) 
    { 
     if (_count >= MAXIMUM) 
      return; 
     _count++; 
    } 
    Task.Run(() => Work()); 
} 

public void CompletedWorkHandler() { 
    lock(_locker_) 
    { 
     _count--; 
    } 
    ... 
} 

Answer 2

Questo è thread-safe, giusto?

Supponiamo che MASSIMO sia uno, il conteggio è zero e cinque thread chiamano CheckForWork.

Tutti e cinque i thread potrebbero verificare che il conteggio sia inferiore a MASSIMO. Il contatore verrebbe quindi espulso fino a cinque e cinque inizi.

Ciò sembra contrario all'intenzione del codice.

Inoltre: il campo non è volatile. Quindi quale meccanismo garantisce che qualsiasi thread legga un valore aggiornato sul percorso di barriera senza memoria? Niente lo garantisce! Crei una barriera di memoria solo se la condizione è falsa.

Più in generale: stai facendo una falsa economia qui. Procedendo con una soluzione a basso numero di blocchi si risparmiano le decine di nanosecondi che sarebbero stati necessari per il blocco non mirato. Basta prendere il lucchetto. Puoi permetterti una dozzina di nanosecondi in più.

E ancora, più in generale: non scrivere codice di basso di blocco a meno che non sei un esperto di architetture di processori e sapere tutte le ottimizzazioni che una CPU è consentito di eseguire su percorsi a bassa-lock. Non sei un esperto. Nemmeno io. Ecco perché Non scrivere il codice di blocco basso.

Answer 3

Definire il thread sicuro.

Se si desidera assicurarsi che _count non sia mai maggiore di MAXIMUM, allora non è stato possibile.

Che cosa si dovrebbe fare è bloccare intorno a quella troppo:

private int _count; 
private object locker = new object(); 

public void CheckForWork() 
{ 
    lock(locker) 
    { 
     if (_count >= MAXIMUM) return; 
     _count++; 
    } 
    Task.Run(() => Work()); 
} 

public void CompletedWorkHandler() 
{ 
    lock(locker) 
    { 
     _count--; 
    } 
    ... 
} 

Si potrebbe anche voler dare un'occhiata a The SemaphoreSlim di classe.

Answer 4

No, quello che hai non è sicuro. Il controllo per vedere se _count >= MAXIMUM potrebbe correre con la chiamata a Interlocked.Increment da un altro thread. Questo è in realtà davvero difficile da risolvere utilizzando tecniche di blocco basso. Per fare in modo che funzioni correttamente, è necessario eseguire una serie di operazioni diverse in modo atomico senza utilizzare un lucchetto. Questa è la parte difficile. La serie di operazioni in questione sono:

• Leggi _count
• prova _count >= MAXIMUM
• prendere una decisione basata su quanto sopra.
• Incremento _count in base alla decisione presa.

Se non fai apparire tutti e 4 questi passaggi atomici, allora ci sarà una condizione di gara. Il modello standard per eseguire un'operazione complessa senza prendere un blocco è il seguente.

public static T InterlockedOperation<T>(ref T location) 
{ 
    T initial, computed; 
    do 
    { 
    initial = location; 
    computed = op(initial); // where op() represents the operation 
    } 
    while (Interlocked.CompareExchange(ref location, computed, initial) != initial); 
    return computed; 
} 

Avviso che cosa sta succedendo. L'operazione viene ripetutamente eseguita fino a quando l'operazione ICX determina che il valore iniziale non è cambiato tra il momento in cui è stato letto per la prima volta e il momento in cui è stato effettuato il tentativo di cambiarlo. Questo è il modello standard e la magia si verifica tutto a causa della chiamata CompareExchange (ICX). Si noti, tuttavia, che ciò non tiene conto dello ABA problem.

Che cosa si potrebbe fare:

Quindi, prendendo il modello di cui sopra e inserirla nel codice si tradurrebbe in questo.

public void CheckForWork() 
{ 
    int initial, computed; 
    do 
    { 
     initial = _count; 
     computed = initial < MAXIMUM ? initial + 1 : initial; 
    } 
    while (Interlocked.CompareExchange(ref _count, computed, initial) != initial); 
    if (replacement > initial) 
    { 
     Task.Run(() => Work()); 
    } 
} 

Personalmente, vorrei punt sulla strategia a basso serratura del tutto. Ci sono diversi problemi con quello che ho presentato sopra.

• Questo potrebbe effettivamente essere più lento di un blocco rigido. Le ragioni sono difficili da spiegare e al di fuori della portata della mia risposta.
• Qualsiasi deviazione da quanto sopra può causare il fallimento del codice. Sì, è davvero così fragile.
• È difficile da capire. Voglio dire guardalo. È brutto.

Quello che dovrebbe fare:

Andando con il percorso di blocco duro il codice potrebbe essere simile.

private object _lock = new object(); 
private int _count; 

public void CheckForWork() 
{ 
    lock (_lock) 
    { 
    if (_count >= MAXIMUM) return; 
    _count++; 
    } 
    Task.Run(() => Work()); 
} 

public void CompletedWorkHandler() 
{ 
    lock (_lock) 
    { 
    _count--; 
    } 
} 

Si noti che questo è molto più semplice e considerevolmente meno incline agli errori. Si può effettivamente scoprire che questo approccio (hard lock) è in realtà più veloce di quello che ho mostrato sopra (low lock). Di nuovo, la ragione è complicata e ci sono tecniche che possono essere utilizzate per accelerare le cose, ma al di fuori dello scopo di questa risposta.

---

Il problema ABA non è realmente un problema in questo caso perché la logica non dipende _count rimanendo invariata. Importa solo che il suo valore è lo stesso in due punti nel tempo, indipendentemente da quello che è successo nel mezzo. In altre parole, il problema può essere ridotto a uno in cui è sembrava come il valore non è cambiato anche se in realtà potrebbe avere.

Answer 5

Questo è quello che Semaphore e SemaphoreSlim sono per:

private readonly SemaphoreSlim WorkSem = new SemaphoreSlim(Maximum); 

public void CheckForWork() { 
    if (!WorkSem.Wait(0)) return; 
    Task.Run(() => Work()); 
} 

public void CompletedWorkHandler() { 
    WorkSem.Release(); 
    ... 
} 

Answer 6

è possibile effettuare le seguenti operazioni se non si desidera bloccare o passare a un semaforo:

if (_count >= MAXIMUM) return; // not necessary but handy as early return 
if(Interlocked.Increment(ref _count)>=MAXIMUM+1) 
{ 
    Interlocked.Decrement(ref _count);//restore old value 
    return; 
} 
Task.Run(() => Work()); 

Incremento restituisce il valore incrementato su cui puoi ricontrollare se _count è inferiore al massimo, se il test fallisce allora ripristino il vecchio valore