Skip to content

Conversation

hebasto
Copy link
Member

@hebasto hebasto commented Apr 4, 2020

While reproducing the bug from #18517, I've noticed that the bitcoind.pid file has already been removed when the bitcoind hangs.

This PR makes Shutdown() keep the bitcoind.pid file available until the end.

@DrahtBot
Copy link
Contributor

DrahtBot commented Apr 5, 2020

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

Copy link
Contributor

@cvengler cvengler left a comment

Choose a reason for hiding this comment

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

Concept ACK with exception to move the done LogPrintf

@hebasto hebasto force-pushed the 20200404-del-pid branch from b2486a3 to 7fcdec0 Compare April 6, 2020 14:14
@hebasto
Copy link
Member Author

hebasto commented Apr 6, 2020

Updated b2486a3 -> 7fcdec0 (pr18526.01 -> pr18526.02, diff):

I would put line 288 at the very end.
Done should always be the last message in a log. Otherwise it would be weird if stuff still follows

Copy link
Contributor

@cvengler cvengler left a comment

Choose a reason for hiding this comment

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

utACK 7fcdec0

@maflcko
Copy link
Member

maflcko commented Apr 6, 2020

ACK 7fcdec0

Copy link
Contributor

@promag promag left a comment

Choose a reason for hiding this comment

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

Code review ACK 7fcdec0.

Copy link
Contributor

@theStack theStack left a comment

Choose a reason for hiding this comment

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

Code review ACK 7fcdec0

@maflcko maflcko merged commit 3347ca4 into bitcoin:master Apr 10, 2020
@hebasto hebasto deleted the 20200404-del-pid branch April 10, 2020 15:17
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Apr 13, 2020
7fcdec0 Remove PID file at the very end (Hennadii Stepanov)

Pull request description:

  While reproducing the bug from bitcoin#18517, I've noticed that the `bitcoind.pid` file has already been removed when the `bitcoind` hangs.

  This PR makes `Shutdown()` keep the `bitcoind.pid` file available until the end.

ACKs for top commit:
  MarcoFalke:
    ACK 7fcdec0
  emilengler:
    utACK 7fcdec0
  promag:
    Code review ACK 7fcdec0.
  theStack:
    Code review ACK 7fcdec0

Tree-SHA512: 9732ef34e137dbee70a06d922b316b8ea7b9a1c959cf8861b6940cd789336dc19ee468a4c3a28d95d1458076a48270c676b0ff27fec30cf57eced6ddab0a2a9b
Fabcien pushed a commit to Bitcoin-ABC/bitcoin-abc that referenced this pull request Jan 14, 2021
Summary:
> While reproducing the bug from #18517, I've noticed that the bitcoind.pid file has already been removed when the bitcoind hangs.
> This PR makes Shutdown() keep the bitcoind.pid file available until the end.

This is a backport of Core [[bitcoin/bitcoin#18526 | PR18526]]

Test Plan: `ninja all check-all`

Reviewers: #bitcoin_abc, majcosta

Reviewed By: #bitcoin_abc, majcosta

Subscribers: majcosta

Differential Revision: https://reviews.bitcoinabc.org/D8910
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Feb 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants