Skip to content

Commit

Permalink
bugfix: node teardown is the last man to go down
Browse files Browse the repository at this point in the history
Warning: during normal execution node teardown must be the
last thing that happens because command requests return
io.Readers, which may still be constructing or processing
their output. The node (and its subservices) is needed for
this. good night and good luck.
  • Loading branch information
jbenet committed Nov 18, 2014
1 parent d93e49e commit eba0599
Showing 1 changed file with 37 additions and 21 deletions.
58 changes: 37 additions & 21 deletions cmd/ipfs2/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ type cmdInvocation struct {
path []string
cmd *cmds.Command
req cmds.Request
node *core.IpfsNode
}

// main roadmap:
Expand All @@ -51,8 +52,9 @@ type cmdInvocation struct {
// - output the response
// - if anything fails, print error, maybe with help
func main() {
var invoc cmdInvocation
var err error
var invoc cmdInvocation
defer invoc.close()

// we'll call this local helper to output errors.
// this is so we control how to print errors in one place.
Expand Down Expand Up @@ -162,6 +164,37 @@ func (i *cmdInvocation) Run() (output io.Reader, err error) {
return res.Reader()
}

func (i *cmdInvocation) constructNode() (*core.IpfsNode, error) {
if i.req == nil {
return nil, errors.New("constructing node without a request")
}

ctx := i.req.Context()
if ctx == nil {
return nil, errors.New("constructing node without a request context")
}

cfg, err := ctx.GetConfig()
if err != nil {
return nil, fmt.Errorf("constructing node without a config: %s", err)
}

// ok everything is good. set it on the invocation (for ownership)
// and return it.
i.node, err = core.NewIpfsNode(cfg, false)
return i.node, err
}

func (i *cmdInvocation) close() {
// let's not forget teardown. If a node was initialized, we must close it.
// Note that this means the underlying req.Context().Node variable is exposed.
// this is gross, and should be changed when we extract out the exec Context.
if i.node != nil {
log.Info("Shutting down node...")
i.node.Close()
}
}

func (i *cmdInvocation) Parse(args []string) error {
var err error

Expand All @@ -180,6 +213,9 @@ func (i *cmdInvocation) Parse(args []string) error {
ctx := i.req.Context()
ctx.ConfigRoot = configPath
ctx.LoadConfig = loadConfig
// this sets up the function that will initialize the node
// this is so that we can construct the node lazily.
ctx.ConstructNode = i.constructNode

// if no encoding was specified by user, default to plaintext encoding
// (if command doesn't support plaintext, use JSON instead)
Expand Down Expand Up @@ -293,29 +329,9 @@ func callCommand(req cmds.Request, root *cmds.Command) (cmds.Response, error) {
} else {
log.Info("Executing command locally")

// this sets up the function that will initialize the node
// this is so that we can construct the node lazily.
ctx := req.Context()

ctx.ConstructNode = func() (*core.IpfsNode, error) {
cfg, err := ctx.GetConfig()
if err != nil {
return nil, err
}
return core.NewIpfsNode(cfg, false)
}

// Okay!!!!! NOW we can call the command.
res = root.Call(req)

// let's not forget teardown. If a node was initialized, we must close it.
// Note that this means the underlying req.Context().Node variable is exposed.
// this is gross, and should be changed when we extract out the exec Context.
node := req.Context().NodeWithoutConstructing()
if node != nil {
log.Info("Shutting down node...")
node.Close()
}
}
return res, nil
}
Expand Down

0 comments on commit eba0599

Please sign in to comment.