log start and end of Shutdown() #3038

Diapolo commented at 1:38 PM on September 28, 2013: none

could be helpful when debugging shutdown related problems

in src/init.cpp:None in 8a840a1a4f outdated

 127 | @@ -127,6 +128,7 @@ void Shutdown()
 128 |      boost::filesystem::remove(GetPidFile());
 129 |      UnregisterAllWallets();
 130 |      delete pwalletMain;
 131 | +    LogPrintf("Shutdown : Processed...\n");

sipa commented at 4:05 PM on September 28, 2013:

Bikeshedding: I think "Processed..." is weird (both the word itself, and the fact that there are ... after it, as there is nothing in progress anymore). How about "Shutdown: done\n"?

jgarzik commented at 4:15 PM on September 28, 2013:

Yes; I strongly prefer a final "Shutdown: done" as the last act of bitcoind, before exit.

In the recent months, the final output of bitcoind has changed several times, and to an avid "tail -f debug.log" watcher such as myself, it continues to be unclear when bitcoind really exits, leading to my habit of "ps ax | grep -w bitcoind" after Ctrl-C'ing the tail(1) process. Our history of crashing/problems at shutdown led to this paranoia. :)

A "shutdown: done" final line would make my brain much happier, and provide a clear message, evident from simple log viewing, that bitcoind exited without crashing.

Diapolo commented at 5:08 PM on September 28, 2013:

Agreed, I'll update the final string in a few minutes.

wtogami commented at 1:29 AM on September 29, 2013: contributor

This looks very helpful in debugging our shutdown issues, although does the completely unrelated spelling fix belong in this PR?

Diapolo commented at 10:11 AM on September 29, 2013: none

@wtogami OT: It has proven to be effective to add small style or spelling fixes to other pulls as otherwise no dev ever ACKs such pulls in a reasonable amount of time.

wtogami commented at 10:32 AM on September 29, 2013: contributor

@Diapolo Certainly we can come up with a fast track procedure to accept obviously correct style/spelling fixes. That would be better than the inability to ask people to keep changesets limited to a particular purpose because we don't do so ourselves.

Diapolo commented at 10:36 AM on September 29, 2013: none

@wtogami I couldn't agree more :).

sipa commented at 1:40 PM on September 29, 2013: member

@Diapolo I often ignore Qt-related pull requests, as I'm not interested or familiar with that code; perhaps several others have the same tendency, which results in fewer people looking at your pull requests. Maybe that gives you an impression that they are overlooked.

I'd rather not mix unrelated typo fixes with actually functional commits though. On the other hand, I think that if I see pure comments/typo changes that seem uncontroverial, I just merge them.

wtogami commented at 9:40 AM on September 30, 2013: contributor

Please remove the spelling fix from this PR and submit it separately, then let's start a policy discussion on the dev list on fast tracking such obviously correct meaningless fixes, pointing at that PR as an example of why it is needed.

Diapolo commented at 7:46 AM on October 1, 2013: none

Updated to reflect our discussion :).

log start and end of Shutdown()

- could be helpful when debugging shutdown related problems

ced3c24816

Diapolo commented at 6:37 AM on October 2, 2013: none

Updated to fix a merge-conflict.

Diapolo commented at 12:27 PM on October 3, 2013: none

Any reason not to merge this?

sipa commented at 12:33 PM on October 3, 2013: member

ACK

jgarzik commented at 1:16 PM on October 3, 2013: contributor

ACK

jgarzik referenced this in commit 5ad5e2d836 on Oct 3, 2013

jgarzik merged this on Oct 3, 2013

jgarzik closed this on Oct 3, 2013

Bushstar referenced this in commit 8d89350b8a on Apr 8, 2020

DrahtBot locked this on Sep 8, 2021