From 65489c1744410850c1c6221ca9c6d42479776515 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Mon, 20 Nov 2017 19:43:11 -0800 Subject: [PATCH 1/2] fix concurrent SetError in pin command (segfault) Also, buffer the response channel. I believe we had a go routine leak here before. License: MIT Signed-off-by: Steven Allen --- core/commands/pin.go | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/core/commands/pin.go b/core/commands/pin.go index 8b23ad6bb..308d2ebf8 100644 --- a/core/commands/pin.go +++ b/core/commands/pin.go @@ -90,15 +90,14 @@ 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) @@ -106,16 +105,16 @@ var addPinCmd = &cmds.Command{ 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()} From d89a4b6960b48c973a59e12b0591e11bb21a4932 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Mon, 20 Nov 2017 19:44:51 -0800 Subject: [PATCH 2/2] fix concurrent SetError in add command I believe this also fixes a potential go routine leak (on race). License: MIT Signed-off-by: Steven Allen --- core/commands/add.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/core/commands/add.go b/core/commands/add.go index d54291687..f23afd4ab 100644 --- a/core/commands/add.go +++ b/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 + } } }()