Don't close old debug log file handle prematurely when trying to re-open (on SIGHUP) by practicalswift · Pull Request #13159 · bitcoin/bitcoin

practicalswift · 2018-05-03T16:09:13Z

Don't close old debug log file handle prematurely when trying to re-open (on SIGHUP).

luke-jr

utACK

…pen (on SIGHUP) Github-Pull: bitcoin#13159 Rebased-From: c4671629c5ee430238222d43f140ca8565c028b1

DrahtBot · 2018-07-22T23:49:35Z

The last travis run for this pull request was 80 days ago and is thus outdated. To trigger a fresh travis build, this pull request should be closed and re-opened.

ajtowns · 2018-08-07T07:32:36Z

Tested ACK 37efe5b7ea059e56d8f5170c91e3160db286cad4

To test:

$ bitcoind -regtest -daemon
$ cd ~/.bitcoin/regtest
$ mv debug.log{,.old}
$ touch debug.log; chmod 400 debug.log
$ killall -HUP bitcoind
$ bitcoin-cli -regtest savemempool

Current behaviour of master is that the "Dumped mempool" message (along with any other subsequent log messages) aren't logged to a file, and further that fixing permissions on debug.log and HUP'ing the process isn't sufficient to re-enable logging to file.

With the patch, logging continues in debug.log.old, and after fixing the permissions HUP'ing the process causes logging to move to the new file.

maflcko · 2018-08-20T18:59:14Z

utACK 37efe5b7ea059e56d8f5170c91e3160db286cad4

…pen (on SIGHUP)

practicalswift · 2018-08-29T14:06:43Z

Rebased!

@luke-jr @ajtowns @MarcoFalke Please re-review :-)

maflcko · 2018-08-29T19:12:28Z

re-utACK 75ea00f

ajtowns · 2018-08-30T09:07:17Z

re-ACK 75ea00f

laanwj · 2018-08-31T11:19:10Z

utACK 75ea00f, good to get rid of the need of freopen I suppose

@ajtowns

…trying to re-open (on SIGHUP) 75ea00f Remove unused fsbridge::freopen (practicalswift) cceedbc Don't close old debug log file handle prematurely when trying to re-open (on SIGHUP) (practicalswift) Pull request description: Don't close old debug log file handle prematurely when trying to re-open (on `SIGHUP`). Context: #13148 (comment) Thanks @ajtowns! Tree-SHA512: c436b4286f00fc428b60269b6d6321f435c72c7ccec3c15b2194aac71196529b30f32c2384b418ffe3ed67ba7ee8ec51f4c9c5748e65945697c0437eafcdacd1

@ajtowns

…trying to re-open (on SIGHUP) Summary: 75ea00f391b742e435c650aae3e827aad913d552 Remove unused fsbridge::freopen (practicalswift) cceedbc4bf1056db17e0adf76d0db45b94777671 Don't close old debug log file handle prematurely when trying to re-open (on SIGHUP) (practicalswift) Pull request description: Don't close old debug log file handle prematurely when trying to re-open (on `SIGHUP`). Context: bitcoin/bitcoin#13148 (comment) Thanks @ajtowns! Tree-SHA512: c436b4286f00fc428b60269b6d6321f435c72c7ccec3c15b2194aac71196529b30f32c2384b418ffe3ed67ba7ee8ec51f4c9c5748e65945697c0437eafcdacd1 Backport of Core PR13159 bitcoin/bitcoin#13159 Depends on D3976 Test Plan: make check ./bitcoind -regtest -daemon cd <path>/.bitcoin/regtest mv debug.log{,.old} touch debug.log; chmod 400 debug.log killall -HUP bitcoind <patch>/bitcoin-abc/build/src/bitcoin-cli -regtest savemempool Verify that the new debug.log file is empty and debug.log.old continued to be written to Reviewers: deadalnix, Fabien, jasonbcox, O1 Bitcoin ABC, #bitcoin_abc Reviewed By: Fabien, O1 Bitcoin ABC, #bitcoin_abc Differential Revision: https://reviews.bitcoinabc.org/D3979

@ajtowns

…trying to re-open (on SIGHUP) Summary: 75ea00f391b742e435c650aae3e827aad913d552 Remove unused fsbridge::freopen (practicalswift) cceedbc4bf1056db17e0adf76d0db45b94777671 Don't close old debug log file handle prematurely when trying to re-open (on SIGHUP) (practicalswift) Pull request description: Don't close old debug log file handle prematurely when trying to re-open (on `SIGHUP`). Context: bitcoin/bitcoin#13148 (comment) Thanks @ajtowns! Tree-SHA512: c436b4286f00fc428b60269b6d6321f435c72c7ccec3c15b2194aac71196529b30f32c2384b418ffe3ed67ba7ee8ec51f4c9c5748e65945697c0437eafcdacd1 Backport of Core PR13159 bitcoin/bitcoin#13159 Depends on D3976 Test Plan: make check ./bitcoind -regtest -daemon cd <path>/.bitcoin/regtest mv debug.log{,.old} touch debug.log; chmod 400 debug.log killall -HUP bitcoind <patch>/bitcoin-abc/build/src/bitcoin-cli -regtest savemempool Verify that the new debug.log file is empty and debug.log.old continued to be written to Reviewers: deadalnix, Fabien, jasonbcox, O1 Bitcoin ABC, #bitcoin_abc Reviewed By: Fabien, O1 Bitcoin ABC, #bitcoin_abc Differential Revision: https://reviews.bitcoinabc.org/D3979

@ajtowns

…trying to re-open (on SIGHUP) Summary: 75ea00f391b742e435c650aae3e827aad913d552 Remove unused fsbridge::freopen (practicalswift) cceedbc4bf1056db17e0adf76d0db45b94777671 Don't close old debug log file handle prematurely when trying to re-open (on SIGHUP) (practicalswift) Pull request description: Don't close old debug log file handle prematurely when trying to re-open (on `SIGHUP`). Context: bitcoin/bitcoin#13148 (comment) Thanks @ajtowns! Tree-SHA512: c436b4286f00fc428b60269b6d6321f435c72c7ccec3c15b2194aac71196529b30f32c2384b418ffe3ed67ba7ee8ec51f4c9c5748e65945697c0437eafcdacd1 Backport of Core PR13159 bitcoin/bitcoin#13159 Depends on D3976 Test Plan: make check ./bitcoind -regtest -daemon cd <path>/.bitcoin/regtest mv debug.log{,.old} touch debug.log; chmod 400 debug.log killall -HUP bitcoind <patch>/bitcoin-abc/build/src/bitcoin-cli -regtest savemempool Verify that the new debug.log file is empty and debug.log.old continued to be written to Reviewers: deadalnix, Fabien, jasonbcox, O1 Bitcoin ABC, #bitcoin_abc Reviewed By: Fabien, O1 Bitcoin ABC, #bitcoin_abc Differential Revision: https://reviews.bitcoinabc.org/D3979

84b4185 [Tests] raise minimum fee in feature_blockindexstats (random-zebra) 39d8a20 [Cleanup] Remove OldSetKeyFromPassphrase/OldEncrypt/OldDecrypt (random-zebra) 613e1da [BUG] Fix sizeof in paymentservertests (random-zebra) 0f3e961 [Cleanup] Remove useless call (random-zebra) b1ca5e0 Remove unused fsbridge::freopen (practicalswift) b3a1d84 Don't close old debug log file handle prematurely when trying to re-open (on SIGHUP) (practicalswift) ecfbcfd [Cleanup] Remove unused function AddInvalidSpendsToMap (random-zebra) 15b018c [Refactor] Use InsecureRand in the unit tests (random-zebra) 81fd84c [Refactoring] Replace risky call to rand() with GetRandInt() (random-zebra) 8ba1978 [Qt] Terminate string *pszExePath after readlink and without using memset (practicalswift) b6cd719 Remove unreachable code (g_rpcSignals.PostCommand) (practicalswift) e4f9ab3 [BUG] Memory leak after new CNode / ConnectNode (random-zebra) 536122b Avoid rollingMinimumFeeRate never being able to decay below half (Alex Morcos) 13cad19 fix a bug if the min fee is 0 for FeeFilterRounder (Alex Morcos) aa832d8 [Refactoring] Dereference before/after null check (random-zebra) 4630b7d [Trivial] Pass big parameters by reference, not value (random-zebra) aa7bc7f [Refactoring] Remove unneeded CScript::IsNormalPaymentScript (random-zebra) 6cf3c8f [Trivial] Unitialized scalar fields and variables (random-zebra) Pull request description: This is a collection of small fixes for the following issues/defects (discovered with the coverity tool): - Big parameters passed by value - calls to rand() function - Dereference before/after null-pointer check - Pointer to local variable out of scope - Resource leak with call to ConnectNode - Non-floating point result - unintended integer division (bitcoin#9288) - String not-null-terminated (bitcoin#11193) - Structurally dead code (bitcoin#9575) - Unchecked boolean return value of functions (GetOp, GetTransaction, TxOutToPublicCoin) - Unitialized pointers and scalar fields - Illegal memory access (bitcoin#13159) - Useless call to pubkey.IsCompressed() - Wrong `sizeof` argument (bitcoin#11024 + revert #494) ACKs for top commit: furszy: Looking good, ACK 84b4185 Fuzzbawls: ACK 84b4185 Tree-SHA512: 4c920a1858ccde65795c090e4becc47c50c0a78db7ab11935b4d3e2bea3f7f1f8ca3b48ce633d8112673092c35ae7cd05c444ed2b16e283a305c765874e55c1c

@ajtowns

…y when trying to re-open (on SIGHUP) 75ea00f Remove unused fsbridge::freopen (practicalswift) cceedbc Don't close old debug log file handle prematurely when trying to re-open (on SIGHUP) (practicalswift) Pull request description: Don't close old debug log file handle prematurely when trying to re-open (on `SIGHUP`). Context: bitcoin#13148 (comment) Thanks @ajtowns! Tree-SHA512: c436b4286f00fc428b60269b6d6321f435c72c7ccec3c15b2194aac71196529b30f32c2384b418ffe3ed67ba7ee8ec51f4c9c5748e65945697c0437eafcdacd1 # Conflicts: # src/logging.cpp

fanquake added the Utils/log/libs label May 3, 2018

practicalswift changed the title ~~Don't close old debug log file handler prematurely when trying to re-open (on SIGHUP)~~ Don't close old debug log file handle prematurely when trying to re-open (on SIGHUP) May 3, 2018

practicalswift force-pushed the handle-reopen-failed branch 2 times, most recently from 2f64058 to 37efe5b Compare May 3, 2018 16:25

luke-jr approved these changes Jun 16, 2018

View reviewed changes

luke-jr pushed a commit to bitcoinknots/bitcoin that referenced this pull request Jul 7, 2018

Don't close old debug log file handle prematurely when trying to re-o…

51d2202

…pen (on SIGHUP) Github-Pull: bitcoin#13159 Rebased-From: c4671629c5ee430238222d43f140ca8565c028b1

DrahtBot closed this Jul 22, 2018

DrahtBot reopened this Jul 22, 2018

This was referenced Jul 26, 2018

[bugfix] Fix encoding issue for Windows #13426

Closed

Test for Windows encoding issue #13787

Merged

This was referenced Aug 3, 2018

utils: drop boost::interprocess::file_lock #13862

Merged

utils: Use _wfopen and _wfreopen on Windows #13866

Merged

utils: Add fstream wrapper to allow to pass unicode filename on Windows #13878

Merged

ajtowns mentioned this pull request Aug 7, 2018

Log early messages with -printtoconsole #13088

Closed

DrahtBot added the Needs rebase label Aug 29, 2018

practicalswift added 2 commits August 29, 2018 16:05

Don't close old debug log file handle prematurely when trying to re-o…

cceedbc

…pen (on SIGHUP)

Remove unused fsbridge::freopen

75ea00f

practicalswift force-pushed the handle-reopen-failed branch from 37efe5b to 75ea00f Compare August 29, 2018 14:06

DrahtBot removed the Needs rebase label Aug 29, 2018

laanwj merged commit 75ea00f into bitcoin:master Aug 31, 2018

Bushstar mentioned this pull request Sep 4, 2018

commits from bitcoin/master FeatherCoin/Feathercoin#359

Merged

random-zebra mentioned this pull request Feb 5, 2021

[Bug][Refactoring] Miscellaneous fixes PIVX-Project/PIVX#2189

Merged

practicalswift deleted the handle-reopen-failed branch April 10, 2021 19:35

str4d mentioned this pull request Dec 6, 2021

Backport changes to src/fs.{cpp,h} zcash/zcash#5406

Open

bitcoin locked as resolved and limited conversation to collaborators Aug 18, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Don't close old debug log file handle prematurely when trying to re-open (on SIGHUP)#13159

Don't close old debug log file handle prematurely when trying to re-open (on SIGHUP)#13159
laanwj merged 2 commits into
bitcoin:masterfrom
practicalswift:handle-reopen-failed

practicalswift commented May 3, 2018 •

edited by DrahtBot

Loading

Uh oh!

luke-jr left a comment

Uh oh!

DrahtBot commented Jul 22, 2018

Uh oh!

ajtowns commented Aug 7, 2018 •

edited

Loading

Uh oh!

maflcko commented Aug 20, 2018

Uh oh!

practicalswift commented Aug 29, 2018

Uh oh!

maflcko commented Aug 29, 2018

Uh oh!

ajtowns commented Aug 30, 2018

Uh oh!

laanwj commented Aug 31, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Uh oh!

Conversation

practicalswift commented May 3, 2018 • edited by DrahtBot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

luke-jr left a comment

Choose a reason for hiding this comment

Uh oh!

DrahtBot commented Jul 22, 2018

Uh oh!

ajtowns commented Aug 7, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maflcko commented Aug 20, 2018

Uh oh!

practicalswift commented Aug 29, 2018

Uh oh!

maflcko commented Aug 29, 2018

Uh oh!

ajtowns commented Aug 30, 2018

Uh oh!

laanwj commented Aug 31, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

practicalswift commented May 3, 2018 •

edited by DrahtBot

Loading

ajtowns commented Aug 7, 2018 •

edited

Loading