@cryptotaxi247 / kubo / commits / eba0599dd

bugfix: node teardown is the last man to go down

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.

Juan Batiz-Benet committed Nov 17, 2014 at 23:12 UTC eba0599dd239d14c94a6e86a189e8d46e3998252
1 file changed +37 -21
cmd/ipfs2/main.go
+37 -21
@@ -42,6 +42,7 @@ type cmdInvocation struct {
42 path []string
43 cmd *cmds.Command
44 req cmds.Request
45 + node *core.IpfsNode
46 }
47
48 // main roadmap:
@@ -51,8 +52,9 @@ type cmdInvocation struct {
52 // - output the response
53 // - if anything fails, print error, maybe with help
54 func main() {
54 - var invoc cmdInvocation
55 var err error
56 + var invoc cmdInvocation
57 + defer invoc.close()
58
59 // we'll call this local helper to output errors.
60 // this is so we control how to print errors in one place.
@@ -162,6 +164,37 @@ func (i *cmdInvocation) Run() (output io.Reader, err error) {
164 return res.Reader()
165 }
166
167 +func (i *cmdInvocation) constructNode() (*core.IpfsNode, error) {
168 + if i.req == nil {
169 + return nil, errors.New("constructing node without a request")
170 + }
171 +
172 + ctx := i.req.Context()
173 + if ctx == nil {
174 + return nil, errors.New("constructing node without a request context")
175 + }
176 +
177 + cfg, err := ctx.GetConfig()
178 + if err != nil {
179 + return nil, fmt.Errorf("constructing node without a config: %s", err)
180 + }
181 +
182 + // ok everything is good. set it on the invocation (for ownership)
183 + // and return it.
184 + i.node, err = core.NewIpfsNode(cfg, false)
185 + return i.node, err
186 +}
187 +
188 +func (i *cmdInvocation) close() {
189 + // let's not forget teardown. If a node was initialized, we must close it.
190 + // Note that this means the underlying req.Context().Node variable is exposed.
191 + // this is gross, and should be changed when we extract out the exec Context.
192 + if i.node != nil {
193 + log.Info("Shutting down node...")
194 + i.node.Close()
195 + }
196 +}
197 +
198 func (i *cmdInvocation) Parse(args []string) error {
199 var err error
200
@@ -180,6 +213,9 @@ func (i *cmdInvocation) Parse(args []string) error {
213 ctx := i.req.Context()
214 ctx.ConfigRoot = configPath
215 ctx.LoadConfig = loadConfig
216 + // this sets up the function that will initialize the node
217 + // this is so that we can construct the node lazily.
218 + ctx.ConstructNode = i.constructNode
219
220 // if no encoding was specified by user, default to plaintext encoding
221 // (if command doesn't support plaintext, use JSON instead)
@@ -293,29 +329,9 @@ func callCommand(req cmds.Request, root *cmds.Command) (cmds.Response, error) {
329 } else {
330 log.Info("Executing command locally")
331
296 - // this sets up the function that will initialize the node
297 - // this is so that we can construct the node lazily.
298 - ctx := req.Context()
299 -
300 - ctx.ConstructNode = func() (*core.IpfsNode, error) {
301 - cfg, err := ctx.GetConfig()
302 - if err != nil {
303 - return nil, err
304 - }
305 - return core.NewIpfsNode(cfg, false)
306 - }
307 -
332 // Okay!!!!! NOW we can call the command.
333 res = root.Call(req)
334
311 - // let's not forget teardown. If a node was initialized, we must close it.
312 - // Note that this means the underlying req.Context().Node variable is exposed.
313 - // this is gross, and should be changed when we extract out the exec Context.
314 - node := req.Context().NodeWithoutConstructing()
315 - if node != nil {
316 - log.Info("Shutting down node...")
317 - node.Close()
318 - }
335 }
336 return res, nil
337 }