Afflitto da bug multithread

25

Nel mio nuovo team che gestisco, la maggior parte del nostro codice è piattaforma, socket TCP e codice di rete http. Tutto il C ++. La maggior parte proveniva da altri sviluppatori che hanno lasciato la squadra. Gli attuali sviluppatori del team sono molto intelligenti, ma per lo più junior in termini di esperienza.

Il nostro problema più grande: bug di concorrenza multi-thread. La maggior parte delle nostre librerie di classi sono scritte per essere asincrone mediante l'uso di alcune classi di thread pool. I metodi sulle librerie di classi accodano spesso tak di esecuzione lunghi nel pool di thread da un thread e quindi i metodi di callback di quella classe vengono richiamati su un thread diverso. Di conseguenza, abbiamo molti bug di casi limite che implicano ipotesi di threading non corrette. Ciò si traduce in bug sottili che vanno oltre le semplici sezioni e blocchi critici per evitare problemi di concorrenza.

Ciò che rende questi problemi ancora più difficili è che i tentativi di correzione sono spesso errati. Alcuni errori che ho osservato nel tentativo del team (o all'interno del codice legacy stesso) includono qualcosa di simile al seguente:

Errore comune n. 1 - Risolvere il problema di concorrenza semplicemente mettendo un blocco attorno ai dati condivisi, ma dimenticando ciò che accade quando i metodi non vengono richiamati in un ordine previsto. Ecco un esempio molto semplice:

void Foo::OnHttpRequestComplete(statuscode status)
{
    m_pBar->DoSomethingImportant(status);
}

void Foo::Shutdown()
{
    m_pBar->Cleanup();
    delete m_pBar;
    m_pBar=nullptr;
}

Quindi ora abbiamo un bug in cui Shutdown potrebbe essere chiamato mentre OnHttpNetworkRequestComplete si sta verificando. Un tester trova il bug, cattura il crash dump e assegna il bug a uno sviluppatore. A sua volta risolve il bug in questo modo.

void Foo::OnHttpRequestComplete(statuscode status)
{
    AutoLock lock(m_cs);
    m_pBar->DoSomethingImportant(status);
}

void Foo::Shutdown()
{
    AutoLock lock(m_cs);
    m_pBar->Cleanup();
    delete m_pBar;
    m_pBar=nullptr;
}

La correzione sopra sembra buona fino a quando non ti accorgi che c'è un caso limite ancora più sottile. Cosa succede se Shutdown viene chiamato prima OnHttpRequestComplete viene richiamato? Gli esempi reali del mio team sono ancora più complessi e i casi limite sono ancora più difficili da individuare durante il processo di revisione del codice.

Common Error # 2 - risolvendo i problemi di deadlock chiudendo ciecamente il lock, aspettando che l'altro thread finisca, quindi reinserendo il lock - ma senza gestire il caso che l'oggetto è stato appena aggiornato dall'altra discussione!

Common Error # 3 - Anche se gli oggetti sono conteggiati, la sequenza di shutdown "rilascia" il suo puntatore. Ma dimentica di aspettare il thread ancora in esecuzione per rilasciare la sua istanza. Pertanto, i componenti vengono arrestati in modo pulito, quindi le callback spurie o tardive vengono richiamate su un oggetto in uno stato che non si aspetta altre chiamate.

Ci sono altri casi limite, ma la linea di fondo è questa:

La programmazione con multithreading è molto semplice, anche per le persone intelligenti.

Mentre percepisco questi errori, passo il tempo a discutere degli errori con ciascuno sviluppatore nello sviluppo di una correzione più appropriata. Ma sospetto che siano spesso confusi su come risolvere ciascun problema a causa dell'enorme quantità di codice legacy che la correzione "giusta" implicherà toccare.

Stiamo per essere spediti presto, e sono sicuro che le patch che applicheremo saranno valide per la prossima versione. In seguito, avremo un po 'di tempo per migliorare il codice base e il refactator dove necessario. Non avremo tempo di riscrivere tutto. E la maggior parte del codice non è poi così male. Ma sto cercando di codice refactoring tale che i problemi di threading possono essere evitati del tutto.

Un approccio che sto considerando è questo. Per ogni funzione di piattaforma significativa, avere un singolo thread dedicato in cui vengono richiamati tutti gli eventi e le chiamate di rete. Simile al threading degli apartment COM in Windows con l'uso di un loop di messaggi. Le lunghe operazioni di blocco possono ancora essere inviate a un thread del pool di lavoro, ma il callback del completamento viene richiamato sulla thread del componente. I componenti potrebbero anche condividere lo stesso thread. Quindi tutte le librerie di classi in esecuzione all'interno del thread possono essere scritte con l'assunzione di un singolo mondo con thread.

Prima di intraprendere questa strada, sono anche molto interessato se ci sono altre tecniche standard o schemi di progettazione per affrontare problemi con multithreading. E devo sottolineare - qualcosa al di là di un libro che descrive le basi dei mutex e dei semafori. Cosa ne pensi?

Sono anche interessato a qualsiasi altro approccio da adottare nei confronti di un processo di refactoring. Compreso uno dei seguenti:

  1. Letteratura o articoli sui modelli di progettazione attorno ai fili. Qualcosa al di là di un'introduzione ai mutex e ai semafori. Non abbiamo nemmeno bisogno di un parallelismo massiccio, solo modi per progettare un modello di oggetto in modo da gestire eventi asincroni da altri thread correttamente .

  2. Modi per diagrammare il threading di vari componenti, in modo che sia facile studiare e sviluppare soluzioni per. (Ovvero, un equivalente UML per discutere i thread tra oggetti e classi)

  3. Formazione del team di sviluppo sui problemi relativi al codice multithread.

  4. Che cosa faresti?

posta koncurrency 25.05.2012 - 09:55
fonte

11 risposte

27

Il tuo codice ha importanti altri problemi oltre a quello. Eliminazione manuale di un puntatore? Chiamando una funzione cleanup ? Owch. Inoltre, come indicato con precisione nel commento della domanda, non utilizzi RAII per il tuo blocco, che è un altro errore piuttosto epico e garantisce che quando DoSomethingImportant genera un'eccezione, accadono cose terribili.

Il fatto che questo bug con multithreading stia succedendo è solo un sintomo del problema principale - il tuo codice ha una pessima semantica in qualsiasi situazione di threading e stai usando completamente strumenti inaffidabili ed ex-idiomi. Se fossi in te, sarei sorpreso che funzioni con un singolo thread, figuriamoci di più.

Common Mistake #3 - Even though the objects are reference counted, the shutdown sequence "releases" it's pointer. But forgets to wait for the thread that is still running to release it's instance. As such, components are shutdown cleanly, then spurious or late callbacks are invoked on an object in an state not expecting any more calls.

L'intero punto di riferimento è che il thread ha già rilasciato la sua istanza . Perché se no, non può essere distrutto perché il thread ha ancora un riferimento.

Utilizza std::shared_ptr . Quando tutti i thread sono stati rilasciati (e nessuno , quindi, può chiamare la funzione, poiché non hanno alcun puntatore), quindi viene chiamato il distruttore. Questo è sicuro.

In secondo luogo, utilizza una vera libreria di threading, come Thread Building Blocks di Intel o Libreria Parallel Patterns di Microsoft. Scrivere il proprio è dispendioso in termini di tempo e inaffidabile e il codice è pieno di dettagli di threading di cui non ha bisogno. Fare le tue serrature è altrettanto brutto di fare la tua gestione della memoria. Hanno già implementato molti linguaggi di threading molto utili generici che funzionano correttamente per il tuo uso.

    
risposta data 25.05.2012 - 11:55
fonte
9

Altri poster hanno commentato bene cosa dovrebbe essere fatto per risolvere i problemi principali. Questo post riguarda il problema più immediato di applicare patch al codice legacy abbastanza bene da permetterti di rifare tutto nel modo giusto. In altre parole, questo non è il modo giusto per fare le cose, è solo un modo per zoppicare per ora.

La tua idea di consolidare gli eventi chiave è un buon inizio. Mi spingerei fino all'utilizzo di un singolo thread di invio per gestire tutti gli eventi di sincronizzazione delle chiavi, ovunque ci sia dipendenza dall'ordine. Impostare una coda di messaggi thread-safe e ovunque si eseguano attualmente operazioni sensibili alla concorrenza (allocazioni, cleanup, callback, ecc.), Invece di inviare un messaggio a quel thread e farlo eseguire o attivare l'operazione. L'idea è che questo thread controlla tutti gli avvii, arresti, allocazioni e ripuliture dell'unità di lavoro.

Il thread di invio non risolve i problemi che hai descritto, li consolida in un unico punto. Devi ancora preoccuparti di eventi / messaggi che si verificano in ordine imprevisto. Gli eventi con tempi di esecuzione significativi dovranno comunque essere inviati ad altri thread, quindi ci sono ancora problemi con la concorrenza sui dati condivisi. Un modo per mitigare questo è evitare di passare dati per riferimento. Quando possibile, i dati nei messaggi di spedizione devono essere copie che saranno di proprietà del destinatario. (Questo è il modo di rendere immutabili i dati che altri hanno menzionato).

Il vantaggio di questo approccio di spedizione è che all'interno del thread di invio si ha una sorta di rifugio sicuro in cui almeno si sa che determinate operazioni si verificano in sequenza. Lo svantaggio è che crea un collo di bottiglia e un sovraccarico della CPU extra. Suggerisco di non preoccuparmi di nessuna di queste cose in un primo momento: concentrarsi su come ottenere una certa misura dell'operazione corretta prima muovendo il più possibile nel thread di invio. Quindi fai un po 'di profilazione per vedere quale è il tempo di CPU più alto e inizia a spostarlo di nuovo fuori dal thread di invio usando le corrette tecniche di multithreading.

Ancora una volta, quello che sto descrivendo non è il modo giusto di fare le cose, ma è un processo che può spingerti verso il modo giusto con incrementi che sono abbastanza piccoli da rispettare le scadenze commerciali.

    
risposta data 25.05.2012 - 17:20
fonte
8

In base al codice visualizzato, hai una pila di WTF. È estremamente difficile, se non impossibile, correggere in modo incrementale un'applicazione multi-threaded scritta male. Dì ai proprietari che l'applicazione non sarà mai affidabile senza una rilavorazione significativa. Fornisci loro una stima basata sull'ispezione e rielaborazione di ogni bit del codice che interagisce con oggetti condivisi. Prima date loro una stima per l'ispezione. Quindi puoi dare una stima per la rilavorazione.

Quando rielaborate il codice, dovreste pianificare di scrivere il codice in modo che sia provabilmente corretto. Se non sai come farlo, trova qualcuno che lo fa o finirai nello stesso posto.

    
risposta data 25.05.2012 - 18:14
fonte
7

Se hai tempo da dedicare al refactoring della tua applicazione, ti consiglierei di dare un'occhiata al modello dell'attore (vedi ad esempio Theron , Casablanca , libcppa , CAF per implementazioni C ++).

Gli attori sono oggetti che vengono eseguiti contemporaneamente e comunicano tra loro solo utilizzando lo scambio di messaggi asincroni. Quindi, tutti i problemi di gestione dei thread, mutex, deadlock, ecc. Sono trattati da una libreria di implementazione degli attori e puoi concentrarti sull'implementazione del comportamento dei tuoi oggetti (attori), che si riduce a ripetere il ciclo

  1. Ricevi messaggio
  2. Esegui calcolo
  3. Invia messaggi / crea / uccidi altri attori.

Un approccio per te potrebbe essere quello di fare un po 'di lettura sull'argomento prima, e possibilmente dare un'occhiata a una libreria o due per vedere se il modello attore può essere integrato nel codice.

Ho usato (una versione semplificata di) questo modello in un mio progetto da alcuni mesi e sono stupito di quanto sia robusto.

    
risposta data 25.05.2012 - 17:55
fonte
6

Common mistake #1 - Fixing concurrency issue by just put a lock around the shared data, but forgetting about what happens when methods don't get called in an expected order. Here's a very simple example:

L'errore qui non è il "dimenticare", ma il "non aggiustarlo". Se ci sono cose che accadono in un ordine inaspettato, hai un problema. Dovresti risolverlo invece di cercare di aggirare il problema (schiaffeggiare un lucchetto su qualcosa di solito è un aggiramento).

Dovresti provare ad adattare il modello / la messaggistica degli attori in una certa misura e ad avere una separazione di interesse. Il ruolo di Foo è chiaramente quello di gestire un qualche tipo di comunicazione HTTP. Se vuoi progettare il tuo sistema in parallelo, è il livello sopra che deve gestire i cicli di vita degli oggetti e la sincronizzazione degli accessi di conseguenza.

Cercare di far funzionare un numero di thread sugli stessi dati mutabili è difficile. Ma è anche raramente necessario. Tutti i casi comuni che richiedono questo, sono già stati estrapolati in concetti più gestibili e implementati un numero di volte per qualsiasi importante linguaggio imperativo. Devi solo usarli.

    
risposta data 25.05.2012 - 13:17
fonte
2

I tuoi problemi sono piuttosto negativi, ma tipici del cattivo uso del C ++. La revisione del codice risolverà alcuni di questi problemi. 30 minuti, un set di bulbi oculari rappresenta il 90% dei risultati. (Citazione per questo è googleable)

Problema n. 1 È necessario assicurarsi che vi sia una stretta gerarchia di blocco per impedire il blocco del deadlock.

Se sostituisci Autolock con un wrapper e una macro puoi farlo.

Mantieni una mappa globale statica dei blocchi creata nel retro del tuo wrapper. È possibile utilizzare una macro per inserire le informazioni relative al nome e al numero di riga nel costruttore del wrapper Autolock.

Avrai anche bisogno di un grafico di dominatore statico.

Ora all'interno del blocco devi aggiornare il grafico del dominatore e, se ricevi una modifica all'ordine, asserisci un errore e interrompi.

Dopo numerosi test potresti liberarti della maggior parte dei deadlock latenti.

Il codice è lasciato come esercizio per lo studente.

Il problema n. 2 andrà quindi via (principalmente)

La tua soluzione archientuale funzionerà. L'ho usato prima nei sistemi mission e life crtical. La mia opinione su questo è

  • Passa oggetti immutabili o creane copie prima di passare.
  • Non condividere dati tramite variabili pubbliche o getter.

  • Gli eventi esterni arrivano tramite una distribuzione multithread in una coda servita da un thread. Ora puoi ordinare il motivo della gestione degli eventi.

  • I dati cambiano che i thread incrociati entrano in un thread safe-safe, vengono gestiti da un thread. Crea abbonamenti. Ora puoi ordinare il motivo dei flussi di dati.

  • Se i tuoi dati devono passare da una città all'altra, pubblicarli nella coda dei dati. Questo lo copierà e lo passerà agli abbonati in modo asincrono. Rompe anche tutte le dipendenze dei dati nel programma.

Questo è praticamente un modello di attore a buon mercato. I collegamenti di Giorgio ti aiuteranno.

Infine, il tuo problema con gli oggetti di arresto.

Quando fai il conteggio dei riferimenti, hai risolto il 50%. L'altro 50% è per richiamare i callback di conteggio. Passa ai titolari di callback un riferimento. La chiamata di spegnimento deve quindi attendere il conteggio zero sul conto. Non risolve i complicati grafici degli oggetti; sta entrando nella vera raccolta dei rifiuti. (Qual è la motivazione in Java per non dare alcuna promessa su quando o se finalize () verrà chiamato, per farti uscire dalla programmazione in questo modo.)

    
risposta data 12.10.2012 - 04:06
fonte
2

Per i futuri esploratori: per completare la risposta sul modello dell'attore vorrei aggiungere CSP ( comunicare i processi sequenziali ) con un cenno alla più ampia famiglia di calcoli di processo. CSP è simile al modello dell'attore, ma si divide in modo diverso. Hai ancora un mucchio di thread, ma comunicano attraverso canali specifici, piuttosto che specificamente l'uno con l'altro, ed entrambi i processi devono essere pronti rispettivamente per inviare e ricevere prima che accada. Esiste anche una lingua formalizzata per la dimostrazione corretta del codice CSP. Sto ancora passando all'utilizzo intensivo di CSP, ma l'ho utilizzato in alcuni progetti per alcuni mesi, ora, ed è molto semplificato.

L'Università del Kent ha un'implementazione in C ++ ( link , clonata su link ).

    
risposta data 04.04.2018 - 18:14
fonte
1

Literature or papers on design patterns around threads. Something beyond an introduction to mutexes and semaphores. We don't need massive parallelism either, just ways to design an object model so as to handle asynchronous events from other threads correctly.

Attualmente sto leggendo questo e spiega tutti i problemi che puoi ottenere e come evitarli, in C ++ (usando la nuova libreria di threading ma penso che le spiegazioni globali siano valide per il tuo caso): link

Ways to diagram the threading of various components, so that it will be easy to study and evolve solutions for. (That is, a UML equivalent for discussing threads across objects and classes)

Personalmente uso un UML semplificato e presumo che i messaggi vengano eseguiti in modo asincrono. Inoltre, questo è vero tra "moduli" ma all'interno di moduli che non voglio sapere.

Educating your development team on the issues with multithreaded code.

Il libro sarebbe d'aiuto, ma penso che gli esercizi / la prototipazione e il mentore esperto sarebbero beter.

What would you do?

Eviterei totalmente che le persone non capiscano che i problemi di concorrenza funzionano sul progetto. Ma immagino che non puoi farlo, quindi nel tuo caso specifico, oltre a cercare di assicurarsi che il team sia più istruito, non ne ho idea.

    
risposta data 25.05.2012 - 10:32
fonte
1

Sei già sulla strada riconoscendo il problema e cercando attivamente una soluzione. Ecco cosa farei:

  • Siediti e progetta un modello di threading per la tua applicazione. Questo è un documento che risponde a domande come: Quali tipi di thread hai? Quali cose dovrebbero essere fatte in quale thread? Quali tipi diversi di schemi di sincronizzazione dovresti usare? In altre parole, dovrebbe descrivere le "regole di ingaggio" quando si affrontano problemi di multithreading.
  • Utilizza gli strumenti di analisi del thread per verificare la presenza di errori nel codebase. Valgrind ha una verifica dei thread chiamata Helgrind che è utile per individuare cose come lo stato condiviso manipolato senza una sincronizzazione corretta. Ci sono sicuramente altri buoni strumenti là fuori, vai a cercarli.
  • Considera la migrazione da C ++. Il C ++ è un incubo per scrivere programmi concorrenti. La mia scelta personale sarebbe Erlang , ma è una questione di gusti.
risposta data 25.05.2012 - 11:44
fonte
1

Osservando il tuo esempio: non appena Foo :: Shutdown inizia a essere eseguito, non deve essere possibile chiamare OnHttpRequestComplete per eseguirlo più. Questo non ha nulla a che fare con alcuna implementazione, semplicemente non può funzionare.

Si potrebbe anche sostenere che Foo :: Shutdown non debba essere richiamabile mentre è in esecuzione una chiamata a OnHttpRequestComplete (sicuramente true) e probabilmente non se una chiamata a OnHttpRequestComplete è ancora in sospeso.

La prima cosa da fare è non bloccare ecc., ma la logica di ciò che è permesso o meno. Un modello semplice potrebbe essere che la classe abbia zero o più richieste incomplete, zero o più completamenti che non sono stati ancora chiamati, zero o più completamenti in esecuzione e che l'oggetto desidera arrestare o meno.

Foo :: L'arresto dovrebbe terminare l'esecuzione di completamenti, per eseguire richieste incomplete al punto in cui possono essere arrestate se possibile, per non consentire l'avvio di ulteriori completamenti, per non consentire l'avvio di più richieste.

Che cosa devi fare: aggiungi specifiche alle tue funzioni dicendo esattamente cosa faranno. (Ad esempio, l'avvio di una richiesta http potrebbe non riuscire dopo l'arresto di Shutdown). E poi scrivi le tue funzioni in modo che soddisfino le specifiche.

I lucchetti vengono utilizzati al meglio solo per il minor tempo possibile per controllare la modifica delle variabili condivise. Quindi potresti avere una variabile "performingShutDown" che è protetta da un Lock.

    
risposta data 06.04.2018 - 15:43
fonte
0

What would you do?

Per essere onesti; Scapperei subito,

I problemi di concorrenza sono NASTY . Qualcosa può funzionare perfettamente per mesi e poi (a causa del tempo specifico di diverse cose) improvvisamente esplodere sul volto del cliente, senza modo di capire cosa è successo, nessuna speranza di vedere mai una bella (riproducibile) segnalazione di bug e in nessun modo per essere sicuro che non si trattasse di un problema hardware che non ha nulla a che fare con il software.

Per evitare problemi di concorrenza è necessario iniziare durante la fase di progettazione, iniziando esattamente da come lo si farà ("ordine di blocco globale", modello di attore, ...). Non è qualcosa che cerchi di risolvere in preda al panico nella speranza che tutto non si autodistrugga dopo un'imminente uscita.

Nota che non sto scherzando qui. Le tue parole (" La maggior parte proviene da altri sviluppatori che hanno lasciato il team. Gli attuali sviluppatori del team sono molto intelligenti, ma per lo più junior in termini di esperienza. ") indicano che tutto il l'esperienza delle persone ha già fatto ciò che sto suggerendo.

    
risposta data 07.04.2018 - 11:31
fonte

Leggi altre domande sui tag