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

pull practicalswift wants to merge 2 commits into bitcoin:master from practicalswift:handle-reopen-failed changing 3 files +5 −11

practicalswift commented at 4:09 PM on May 3, 2018: contributor

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

Context: #13148 (comment)

Thanks @ajtowns!
fanquake added the label Utils/log/libs on May 3, 2018
practicalswift renamed this:
~~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)
on May 3, 2018
practicalswift force-pushed on May 3, 2018
practicalswift force-pushed on May 3, 2018
luke-jr approved
luke-jr commented at 4:10 AM on June 16, 2018: member

utACK
luke-jr referenced this in commit 51d2202952 on Jul 7, 2018
DrahtBot commented at 11:49 PM on July 22, 2018: contributor

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.
DrahtBot closed this on Jul 22, 2018
DrahtBot reopened this on Jul 22, 2018
DrahtBot cross-referenced this on Jul 26, 2018 from issue [bugfix] Fix encoding issue for Windows by ken2812221
DrahtBot cross-referenced this on Jul 28, 2018 from issue Test for Windows encoding issue by ken2812221
DrahtBot cross-referenced this on Aug 3, 2018 from issue utils: drop boost::interprocess::file_lock by ken2812221
DrahtBot cross-referenced this on Aug 3, 2018 from issue utils: Use _wfopen and _wfreopen on Windows by ken2812221
DrahtBot cross-referenced this on Aug 4, 2018 from issue utils: Add fstream wrapper to allow to pass unicode filename on Windows by ken2812221
ajtowns commented at 7:32 AM on August 7, 2018: contributor
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.
ajtowns cross-referenced this on Aug 7, 2018 from issue Log early messages with -printtoconsole by ajtowns
MarcoFalke commented at 6:59 PM on August 20, 2018: member

utACK 37efe5b7ea059e56d8f5170c91e3160db286cad4
DrahtBot added the label Needs rebase on Aug 29, 2018
Don't close old debug log file handle prematurely when trying to re-open (on SIGHUP) cceedbc4bf
Remove unused fsbridge::freopen 75ea00f391
practicalswift force-pushed on Aug 29, 2018
practicalswift commented at 2:06 PM on August 29, 2018: contributor

Rebased! @luke-jr @ajtowns @MarcoFalke Please re-review :-)
DrahtBot removed the label Needs rebase on Aug 29, 2018
MarcoFalke commented at 7:12 PM on August 29, 2018: member

re-utACK 75ea00f391b742e435c650aae3e827aad913d552
ajtowns commented at 9:07 AM on August 30, 2018: contributor

re-ACK 75ea00f391b742e435c650aae3e827aad913d552
laanwj commented at 11:19 AM on August 31, 2018: member

utACK 75ea00f391b742e435c650aae3e827aad913d552, good to get rid of the need of freopen I suppose
laanwj merged this on Aug 31, 2018
laanwj closed this on Aug 31, 2018
laanwj referenced this in commit bb0277819a on Aug 31, 2018
Bushstar cross-referenced this on Sep 4, 2018 from issue commits from bitcoin/master by Bushstar
jasonbcox referenced this in commit 3bdbe48216 on Sep 13, 2019
jonspock referenced this in commit 130286a16d on Dec 22, 2019
proteanx referenced this in commit 910826634c on Dec 23, 2019
random-zebra cross-referenced this on Feb 5, 2021 from issue [Bug][Refactoring] Miscellaneous fixes by random-zebra
Fuzzbawls referenced this in commit 71ed429370 on Feb 14, 2021
practicalswift deleted the branch on Apr 10, 2021
Munkybooty referenced this in commit a58bf3ca3f on Jun 30, 2021
Munkybooty referenced this in commit 561093f425 on Jul 2, 2021
Munkybooty referenced this in commit 2be36d4db5 on Jul 2, 2021
str4d cross-referenced this on Dec 6, 2021 from issue Backport changes to `src/fs.{cpp,h}` by str4d
bitcoin locked this on Aug 18, 2022

Contributors

Labels

Linked (view graph)

#13088 Log early messages with -printtoconsole #13148 logging: Fix potential use-after-free in LogPrintStr(...)