Merge pull request #4406 from ipfs/fix/4405

Fix two race conditions (and possibly go routine leaks) in commands

Author: whyrusleeping

core/commands/add.go

@@ -342,6 +342,8 @@ You can now check what blocks have been created by:
 	},
 	PostRun: map[cmds.EncodingType]func(cmds.Request, cmds.ResponseEmitter) cmds.ResponseEmitter{
 		cmds.CLI: func(req cmds.Request, re cmds.ResponseEmitter) cmds.ResponseEmitter {
+			ctx := req.Context()
+
 			reNext, res := cmds.NewChanResponsePair(req)
 			outChan := make(chan interface{})
 
@@ -429,9 +431,6 @@ You can now check what blocks have been created by:
 							bar.ShowBar = true
 							bar.ShowTimeLeft = true
 						}
-					case <-req.Context().Done():
-						re.SetError(req.Context().Err(), cmdkit.ErrNormal)
-						return
 					}
 				}
 			}
@@ -469,7 +468,12 @@ You can now check what blocks have been created by:
 						return
 					}
 
-					outChan <- v
+					select {
+					case outChan <- v:
+					case <-ctx.Done():
+						re.SetError(ctx.Err(), cmdkit.ErrNormal)
+						return
+					}
 				}
 			}()
 

core/commands/pin.go

@@ -90,32 +90,31 @@ var addPinCmd = &cmds.Command{
 		v := new(dag.ProgressTracker)
 		ctx := v.DeriveContext(req.Context())
 
-		ch := make(chan []*cid.Cid)
+		type pinResult struct {
+			pins []*cid.Cid
+			err  error
+		}
+		ch := make(chan pinResult, 1)
 		go func() {
-			defer close(ch)
 			added, err := corerepo.Pin(n, ctx, req.Arguments(), recursive)
-			if err != nil {
-				res.SetError(err, cmdkit.ErrNormal)
-				return
-			}
-			ch <- added
+			ch <- pinResult{pins: added, err: err}
 		}()
 
 		ticker := time.NewTicker(500 * time.Millisecond)
 		defer ticker.Stop()
 		defer close(out)
 		for {
 			select {
-			case val, ok := <-ch:
-				if !ok {
-					// error already set just return
+			case val := <-ch:
+				if val.err != nil {
+					res.SetError(val.err, cmdkit.ErrNormal)
 					return
 				}
 
 				if pv := v.Value(); pv != 0 {
 					out <- &AddPinOutput{Progress: v.Value()}
 				}
-				out <- &AddPinOutput{Pins: cidsToStrings(val)}
+				out <- &AddPinOutput{Pins: cidsToStrings(val.pins)}
 				return
 			case <-ticker.C:
 				out <- &AddPinOutput{Progress: v.Value()}