Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The MFS root state isn't saved if the daemon is stopped with Ctrl+C #5868

Closed
schomatis opened this issue Dec 21, 2018 · 3 comments · Fixed by ipfs/go-blockservice#15
Closed
Assignees
Labels
kind/bug A bug in existing code (including security flaws) topic/commands Topic commands topic/MFS Topic MFS

Comments

@schomatis
Copy link
Contributor

Following a series of (maybe not bugs but at least) some unusual behaviors I've found that Ctrl+Cing the daemon causes the MFS root's Close method (in teardown) to silently fail and thus causing any un-flushed files to be lost. Since the ipfs files command has the --flush option set by default this may not have had serious repercussions.

The Close error was (silently) failing because, when flushing, the DAG.Add operation was failing with blockservice is closed that was actually a hard-coded error (which threw me off substantially) shadowing any general HasBlock errors in the exchange service, i.e., Bitswap.

The real error was bitswap is closed generated in receiveBlockFrom that detected that the process was on its way to shutting down. This process was tied to the context that created the Bitswap interface, which actually seems to be derived from the original context that started the daemon that is closed when signaling the interrupt generated by Ctrl+C,

https://github.com/ipfs/go-ipfs/blob/c17aaa6e26512b6bbd0f60c0c39c82daacfba9e1/cmd/ipfs/main.go#L396-L409

This doesn't seem to happen with the ipfs shutdown command which more gracefully issues a Process().Close() without canceling the root context.

@schomatis schomatis added topic/commands Topic commands topic/MFS Topic MFS labels Dec 21, 2018
@Stebalien
Copy link
Member

Hello: #4623

Would ignoring this error be enough?

@schomatis
Copy link
Contributor Author

Would ignoring this error be enough?

I guess so (I'll test it a bit) but the bigger issue I think is that we're not honoring the teardown order

https://github.com/ipfs/go-ipfs/blob/c17aaa6e26512b6bbd0f60c0c39c82daacfba9e1/core/core.go#L668-L676

so any other service inter-dependency we rely on in the future will be subjected to a similar problem.

@Stebalien
Copy link
Member

I see. Yeah, we really need a proper service management system (#5810). Regardless, we shouldn't be using context for closing processes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/bug A bug in existing code (including security flaws) topic/commands Topic commands topic/MFS Topic MFS
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants