Skip to content

Commit 7083445

Browse files
committed
Safe yet correct readpipe closer
1 parent ccbd08b commit 7083445

File tree

1 file changed

+31
-14
lines changed

1 file changed

+31
-14
lines changed

core/commands/dag/dag.go

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -275,17 +275,17 @@ The output of blocks happens in strict DAG-traversal, first-seen, order.
275275
},
276276
Run: func(req *cmds.Request, res cmds.ResponseEmitter, env cmds.Environment) error {
277277

278-
c, err := cid.Decode(req.Arguments[0])
279-
if err != nil {
278+
c, cidErr := cid.Decode(req.Arguments[0])
279+
if cidErr != nil {
280280
return fmt.Errorf(
281281
"unable to parse root specification (currently only bare CIDs are supported): %s",
282-
err,
282+
cidErr,
283283
)
284284
}
285285

286-
node, err := cmdenv.GetNode(env)
287-
if err != nil {
288-
return err
286+
node, nodeErr := cmdenv.GetNode(env)
287+
if nodeErr != nil {
288+
return nodeErr
289289
}
290290

291291
// Code disabled until descent-issue in go-ipld-prime is fixed
@@ -307,6 +307,24 @@ The output of blocks happens in strict DAG-traversal, first-seen, order.
307307
// if err := car.Write(pipeW); err != nil {}
308308

309309
pipeR, pipeW := io.Pipe()
310+
// this is so convoluted because we:
311+
// - want to close the readpipe on error so that the goroutine dies
312+
// - can not do so on success, because during a no-deamon process
313+
// scenario the PostRun will not be able to grab the EOF from the
314+
// now-closed pipe
315+
//
316+
// Skipping this check results in:
317+
//
318+
// cmd/ipfs/ipfs dag export QmdmQXB2mzChmMeKY47C43LxUdg1NDJ5MWcKMKxDu7RgQm >/dev/null
319+
// 0s 0 B / ? [------------------------------------------------------------------------=]Error: io: read/write on closed pipe
320+
// 1s 107.08 MiB / ? [-----------------------------------------=----------] 106.59 MiB/s
321+
//
322+
var emittingError error
323+
defer func() {
324+
if emittingError != nil {
325+
pipeR.Close()
326+
}
327+
}()
310328

311329
errCh := make(chan error, 2) // we only report the 1st error
312330
go func() {
@@ -330,21 +348,20 @@ The output of blocks happens in strict DAG-traversal, first-seen, order.
330348
}
331349
}()
332350

333-
if err := res.Emit(pipeR); err != nil {
334-
pipeR.Close() // ignore the error if any
335-
return err
351+
if emittingError = res.Emit(pipeR); emittingError != nil {
352+
return emittingError
336353
}
337354

338-
err = <-errCh
355+
emittingError = <-errCh
339356

340357
// minimal user friendliness
341-
if err != nil &&
358+
if emittingError != nil &&
342359
!node.IsOnline &&
343-
err == ipld.ErrNotFound {
344-
err = fmt.Errorf("%s (currently offline, perhaps retry after attaching to the network)", err)
360+
emittingError == ipld.ErrNotFound {
361+
emittingError = fmt.Errorf("%s (currently offline, perhaps retry after attaching to the network)", emittingError)
345362
}
346363

347-
return err
364+
return emittingError
348365
},
349366
PostRun: cmds.PostRunMap{
350367
cmds.CLI: func(res cmds.Response, re cmds.ResponseEmitter) error {

0 commit comments

Comments
 (0)