Skip to content

Wait before shutdown to avoid the crash - #593

Merged
sgkim126 merged 1 commit into
CodeChain-io:masterfrom
sgkim126:time
Aug 17, 2018
Merged

sgkim126 merged 1 commit into
CodeChain-io:masterfrom
sgkim126:time

Conversation

@sgkim126

Copy link
Copy Markdown
Contributor

This patch is the hotfix for #348.
I can't find the exact reason for the SIGINT yet, but this patch fixes
the crash.

@sgkim126 sgkim126 added the bug Something isn't working label Aug 16, 2018
@sgkim126
sgkim126 requested a review from kseo August 16, 2018 08:49
@sgkim126 sgkim126 added this to the Testnet milestone Aug 16, 2018
Comment thread codechain/codechain.yml Outdated
long: chain
help: Set the blockchain type out of solo, solo_authority, tendermint, cuckoo, blake_pow, husky or a path to chain scheme file.
takes_value: true
- wait-before-shutdown:

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need a command line option here because this PR is just a hotfix for the crash. Just use a hardcoded constant which is large enough to avoid the crash.

@sgkim126 sgkim126 Aug 17, 2018 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is to make it easy to test.
I'm not sure waiting 1 second is enough to avoid the crash. It was the experimental result on my machine.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we need internal command line options to help test and debug.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not a good idea to expose a command line option that is used only for test & debug purposes. What about adding an environment variable instead? Users don't know what environment variables exist, but we can use them for test & debug purposes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay. I agree with you. I'll change it.

This patch is the hotfix for #348.
I can't find the exact reason for the SIGINT yet, but this patch fixes
the crash.
@sgkim126

Copy link
Copy Markdown
Contributor Author

@kseo PR updated.

@kseo kseo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok

@sgkim126
sgkim126 merged commit 05af3ac into CodeChain-io:master Aug 17, 2018
@sgkim126
sgkim126 deleted the time branch August 17, 2018 05:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants