Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Fix daemonize #10000

Merged
merged 2 commits into from
Nov 30, 2018
Merged

Fix daemonize #10000

merged 2 commits into from
Nov 30, 2018

Conversation

ordian
Copy link
Collaborator

@ordian ordian commented Nov 30, 2018

Closes #9991. (Sorry, I screwed up in #9946 (review), but it didn't segfault for me)

Nice find by @andresilva:

I don't know how rust handles file descriptors and forks
they're probably closed before forking?
rust-lang/rust#24034
so this means any file descriptor is closed before forking/exec I think

The first commit moves fork instruction just before we open the db (so #9367 remains fixed).
The second commit switches daemonize to crates.io version (@tomaka is paritytech/daemonize#1 no longer relevant (cf. knsd/daemonize@4c5f9d0#diff-49a4b4d69f811d5619c481802227b86e)?), it's somewhat unrelated, happy to revert if needed.

@ordian ordian added A0-pleasereview 🤓 Pull request needs code review. B1-patch-beta 🕷🕷 M4-core ⛓ Core client code / Rust. A6-revertrevert 💀 Pull request that reverts recent changes. M5-dependencies 🖇 Dependencies. B0-patch-stable 🕷 Pull request should also be back-ported to the stable branch. labels Nov 30, 2018
@ordian ordian added this to the 2.3 milestone Nov 30, 2018
This was referenced Nov 30, 2018
@tomaka
Copy link
Contributor

tomaka commented Nov 30, 2018

is paritytech/daemonize#1 no longer relevant (cf. knsd/daemonize@4c5f9d0#diff-49a4b4d69f811d5619c481802227b86e)?

Fine by me. If I remember correctly, the intent of my PR was to make it compile on Android.

@5chdn 5chdn added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Nov 30, 2018
@ordian ordian added the A1-onice 🌨 Pull request is reviewed well, but should not yet be merged. label Nov 30, 2018
@5chdn 5chdn merged commit 7000c39 into master Nov 30, 2018
@5chdn 5chdn deleted the make-daemonize-great-again branch November 30, 2018 11:39
@5chdn
Copy link
Contributor

5chdn commented Nov 30, 2018

Noooo I didn't see the on-ice label

@5chdn
Copy link
Contributor

5chdn commented Nov 30, 2018

What's up?

@ordian
Copy link
Collaborator Author

ordian commented Nov 30, 2018

Sorry, @andresilva spotted a problem, I'll open a separate PR.

@5chdn
Copy link
Contributor

5chdn commented Nov 30, 2018

Ok, sorry.

5chdn pushed a commit that referenced this pull request Nov 30, 2018
* Revert "prevent silent errors in daemon mode, closes #9367 (#9946)"

This reverts commit 52d5278.

* deps(daemonize): switch back to crates.io
5chdn pushed a commit that referenced this pull request Nov 30, 2018
* Revert "prevent silent errors in daemon mode, closes #9367 (#9946)"

This reverts commit 52d5278.

* deps(daemonize): switch back to crates.io
5chdn added a commit that referenced this pull request Nov 30, 2018
* version: bump stable to 2.1.8

* Fix Bloom migration (#9992)

* Fix wrong block number in blooms migration

* Fix wrong `const` type (usize -> u64) 😬

* Fix daemonize (#10000)

* Revert "prevent silent errors in daemon mode, closes #9367 (#9946)"

This reverts commit 52d5278.

* deps(daemonize): switch back to crates.io

* move daemonize before creating account provider (#10003)

* move daemonize before creating account provider

* daemonize: add a future-proofing comment
5chdn added a commit that referenced this pull request Nov 30, 2018
* version: bump beta to 2.2.3

* Fix Bloom migration (#9992)

* Fix wrong block number in blooms migration

* Fix wrong `const` type (usize -> u64) 😬

* Fix daemonize (#10000)

* Revert "prevent silent errors in daemon mode, closes #9367 (#9946)"

This reverts commit 52d5278.

* deps(daemonize): switch back to crates.io

* move daemonize before creating account provider (#10003)

* move daemonize before creating account provider

* daemonize: add a future-proofing comment
niklasad1 pushed a commit that referenced this pull request Dec 16, 2018
* Revert "prevent silent errors in daemon mode, closes #9367 (#9946)"

This reverts commit 52d5278.

* deps(daemonize): switch back to crates.io
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A1-onice 🌨 Pull request is reviewed well, but should not yet be merged. A6-revertrevert 💀 Pull request that reverts recent changes. A8-looksgood 🦄 Pull request is reviewed well. B0-patch-stable 🕷 Pull request should also be back-ported to the stable branch. M4-core ⛓ Core client code / Rust. M5-dependencies 🖇 Dependencies.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2.2.2-beta segfaulting in libpthread and corrupting DB when started as daemon
4 participants