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: member

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
ajtowns commented at 7:32 AM on August 7, 2018: member
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.
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: member

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
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
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
DrahtBot locked this on Aug 18, 2022

Contributors

Labels

Linked (view graph)

#13148 logging: Fix potential use-after-free in LogPrintStr(...)