2016-04-04 17 views
5

Ho alcuni problemi con il seguente codice:miglior modo di usare sync.WaitGroup con funzione esterna

package main 

import (
"fmt" 
"sync" 
) 
// This program should go to 11, but sometimes it only prints 1 to 10. 
func main() { 
    ch := make(chan int) 
    var wg sync.WaitGroup 
    wg.Add(2) 
    go Print(ch, wg) // 
    go func(){ 

     for i := 1; i <= 11; i++ { 
      ch <- i 
     } 

     close(ch) 
     defer wg.Done() 


    }() 

    wg.Wait() //deadlock here 
} 

// Print prints all numbers sent on the channel. 
// The function returns when the channel is closed. 
func Print(ch <-chan int, wg sync.WaitGroup) { 
    for n := range ch { // reads from channel until it's closed 
     fmt.Println(n) 
    } 
    defer wg.Done() 
} 

ottengo una situazione di stallo nel luogo specificato. Ho provato a impostare wg.Add(1) invece di 2 e risolve il mio problema. La mia convinzione è che non sto inviando il canale come argomento alla funzione Printer. C'è un modo per farlo? In caso contrario, una soluzione al mio problema sta sostituendo la linea go Print(ch, wg) con:

go func() { 
Print(ch) 
defer wg.Done() 
} 

e modifica della funzione Printer a:

func Print(ch <-chan int) { 
    for n := range ch { // reads from channel until it's closed 
     fmt.Println(n) 
    } 

} 

Qual è la soluzione migliore?

risposta

10

Beh, in primo luogo il vostro errore vero è che si sta dando il metodo Print una copia del sync.WaitGroup, in modo da non chiamare il metodo Done() da un sei Wait() ing on.

Prova a modificare:

package main 

import (
    "fmt" 
    "sync" 
) 

func main() {  
    ch := make(chan int) 

    var wg sync.WaitGroup 
    wg.Add(2)  

    go Print(ch, &wg) 

    go func() { 
     for i := 1; i <= 11; i++ { 
      ch <- i 
     } 
     close(ch) 
     defer wg.Done() 
    }()   

    wg.Wait() //deadlock here 
}     

func Print(ch <-chan int, wg *sync.WaitGroup) { 
    for n := range ch { // reads from channel until it's closed 
     fmt.Println(n) 
    }    
    defer wg.Done() 
} 

Ora, cambiando il metodo di Print per rimuovere il WaitGroup di esso è una buona idea in generale: il metodo non ha bisogno di sapere qualcosa è in attesa che finisca il suo lavoro .

+1

capito, non ho conosciuto che avete l'indirizzo di essere inviato a 'Print' invece che al' WaitGroup' stesso. Sono d'accordo che il metodo non ha bisogno di conoscere il 'WaitGroup', ma supponiamo che lo voglio comunque. Cosa fa allora la stella? Seleziona l'ATTUALE 'WaitGrooup' che ho definito in' main'? E perché questo non è una copia? – Sahand

+0

Il '* sync.WaitGroup' dice al compilatore che vuoi un puntatore a un' WaitGroup' invece di un 'WaitGroup'. Quindi il compilatore si aspetta che tu chiami il metodo dandogli un indirizzo, quindi '& wg'. – Elwinar

+2

Non pensare di poter rimuovere il waitgroup di stampa come wg principale. L'attesa potrebbe passare prima che venga stampato l'ultimo valore se si è solo aspettato il completamento del mittente. Inoltre, non c'è motivo per terminare una funzione con un defero wg.Done(), differire sostanzialmente significa, eseguirlo alla fine. rilascia il differimento –

1

Sono d'accordo con la soluzione di @ Elwinar, che il problema principale nel codice è causato passando una copia del Waitgroup alla funzione Print.

Ciò significa che lo wg.Done() viene utilizzato su una copia di wg definita in main. Pertanto, wg nel numero main non è stato possibile ridurre e quindi si verifica un deadlock quando si utilizza il valore wg.Wait() principale.

Dal momento che si sta anche chiedendo circa la migliore pratica, potrei darvi alcuni suggerimenti del mio:

  • Non rimuovere defer wg.Done() in Print. Poiché la tua goroutine in main è un mittente, e print è un ricevitore, rimuovere wg.Done() nella routine del ricevitore causerà un ricevitore non finito. Questo perché solo il tuo mittente è sincronizzato con il tuo principale, quindi dopo che il tuo mittente è finito, il tuo main è terminato, ma è possibile che il ricevitore funzioni ancora. Il mio punto è: non lasciare qualche goroutina penzolante intorno dopo che la tua routine principale è finita. Chiudili o aspettali.

  • Ricordarsi di eseguire il recupero dal panico ovunque, in particolare la goroutine anonima. Ho visto molti programmatori golan dimenticarsi di mettere il recupero dal panico nelle goroutine, anche se si ricordano di recuperare in normali funzioni. È fondamentale quando vuoi che il tuo codice si comporti correttamente o almeno con garbo quando succede qualcosa di inaspettato.

  • Usa defer prima di ogni telefonata in critiche, come sync chiamate correlate, all'inizio dal momento che non si sa dove il codice potrebbe rompersi. Supponiamo che tu abbia rimosso defer prima del wg.Done() e che si verifichi un panico nella tua goroutine anonima nel tuo esempio.Se non hai il recupero dal panico, andrà nel panico. Ma cosa succede se si recupera il panico? Va tutto bene adesso? No. Avrai lo stallo a wg.Wait() dato che il tuo wg.Done() viene saltato a causa del panico! Tuttavia, utilizzando defer, questo wg.Done() verrà eseguito alla fine, anche se il panico si è verificato. Inoltre, differire prima di close è importante, poiché il suo risultato influenza anche la comunicazione.

ecco il codice modificato in base ai punti che ho citato sopra:

package main 

import (
    "fmt" 
    "sync" 
) 

func main() { 
    ch := make(chan int) 
    var wg sync.WaitGroup 
    wg.Add(2) 
    go Print(ch, &wg) 
    go func() { 

     defer func() { 
      if r := recover(); r != nil { 
       println("panic:" + r.(string)) 
      } 
     }() 

     defer func() { 
      wg.Done() 
     }() 

     for i := 1; i <= 11; i++ { 
      ch <- i 

      if i == 7 { 
       panic("ahaha") 
      } 
     } 

     println("sender done") 
     close(ch) 
    }() 

    wg.Wait() 
} 

func Print(ch <-chan int, wg *sync.WaitGroup) { 
    defer func() { 
     if r := recover(); r != nil { 
      println("panic:" + r.(string)) 
     } 
    }() 

    defer wg.Done() 

    for n := range ch { 
     fmt.Println(n) 
    } 
    println("print done") 
} 

Speranza che aiuta :)

+0

Grazie mille amico! – Sahand

+0

Sono completamente in disaccordo con il messaggio "Non rimuovere il wg da Print_". Ma forse la mia proposta non è stata capita correttamente, quello che consiglio è di fare qualcosa di simile: http://play.golang.org/p/lOopY0bYHT In questo modo, la tua routine non sa che è sincronizzata, che lo rende più semplice. Come regola generale, * a meno che la sincronizzazione non sia parte dell'algoritmo, il tuo codice non dovrebbe mai esserne a conoscenza *. Significa che un metodo che stampa qualcosa sullo schermo non dovrebbe mai richiedere alcuna consapevolezza della sincronizzazione. – Elwinar

+0

È una buona idea per riprendersi dal panico, ma in questo esempio lo sta esagerando. L'unica cosa che può farsi prendere dal panico è l'invio a un canale chiuso, e tu sei quello che lo chiude. Non cadere nella trappola di controllare ogni cosa ogni volta, questo farà solo gonfiare il tuo codice con controlli non necessari e renderà difficile la lettura. – Elwinar

Problemi correlati