Improve Closing (#559)

- Prevent double-closing in cmd/torrent
- Move async closing from storage to torrents, and wait on them to finish tidying up before returning/exiting.

client.go

@@ -411,11 +411,13 @@ func (cl *Client) eachDhtServer(f func(DhtServer)) {
 // Stops the client. All connections to peers are closed and all activity will
 // come to a halt.
 func (cl *Client) Close() {
+	var closeGroup sync.WaitGroup //WaitGroup for any concurrent cleanup to complete before returning.
+	defer closeGroup.Wait() //defer is LIFO. We want to Wait() after cl.unlock()
 	cl.lock()
 	defer cl.unlock()
 	cl.closed.Set()
 	for _, t := range cl.torrents {
-		t.close()
+		t.close(&closeGroup)
 	}
 	for i := range cl.onClose {
 		cl.onClose[len(cl.onClose)-1-i]()
@@ -1252,7 +1254,9 @@ func (cl *Client) dropTorrent(infoHash metainfo.Hash) (err error) {
 		err = fmt.Errorf("no such torrent")
 		return
 	}
-	err = t.close()
+	var wg sync.WaitGroup
+	defer wg.Wait()
+	err = t.close(&wg)
 	if err != nil {
 		panic(err)
 	}

cmd/torrent/main.go

@@ -11,6 +11,7 @@ import (
 	"os"
 	"os/signal"
 	"strings"
+	"sync"
 	"syscall"
 	"time"
 
@@ -332,11 +333,12 @@ func downloadErr() error {
 	if err != nil {
 		return xerrors.Errorf("creating client: %v", err)
 	}
-	defer client.Close()
+	var clientClose sync.Once //In certain situations, close was being called more than once.
+	defer clientClose.Do(client.Close)
 	go exitSignalHandlers(&stop)
 	go func() {
 		<-stop.C()
-		client.Close()
+		clientClose.Do(client.Close)
 	}()
 
 	// Write status on the root path on the default HTTP muxer. This will be bound to localhost

torrent.go

@@ -789,14 +789,19 @@ func (t *Torrent) numPiecesCompleted() (num pieceIndex) {
 	return pieceIndex(t._completedPieces.GetCardinality())
 }
 
-func (t *Torrent) close() (err error) {
+func (t *Torrent) close(wg *sync.WaitGroup) (err error) {
 	t.closed.Set()
 	if t.storage != nil {
+		wg.Add(1)
 		go func() {
+			defer wg.Done()
 			t.storageLock.Lock()
 			defer t.storageLock.Unlock()
 			if f := t.storage.Close; f != nil {
-				f()
+				err1 := f()
+				if err1 != nil {
+					t.logger.WithDefaultLevel(log.Warning).Printf("error closing storage: %v", err1)
+				}
 			}
 		}()
 	}