wwbmmm commented on code in PR #3687:
URL: https://github.com/apache/brpc/pull/3687#discussion_r4500926245
##########
src/butil/logging.cc:
##########
@@ -437,14 +674,25 @@ void Log2File(const std::string& log) {
// thread at the beginning of execution.
LoggingLock::Init(LOCK_LOG_FILE, nullptr);
LoggingLock logging_lock;
- if (InitializeLogFileHandle()) {
+ // Rotate before writing so that a single log is never split into two
files.
+ MaybeRotateLogFileUnlocked(log.size());
+ // InitializeLogFileHandle() also succeeds without opening anything when
+ // the destination no longer logs to a file. That is the case of an async
+ // log that was queued before a later InitLogging() moved the destination
+ // away from the file: there is no handle to write through, so the log is
+ // dropped instead of being written through a null handle, which used to
+ // crash the process. MaybeRotateLogFileUnlocked() above checks the same.
+ if (InitializeLogFileHandle() && log_file) {
#if defined(OS_WIN)
SetFilePointer(log_file, 0, 0, SEEK_END);
- DWORD num_written;
+ DWORD num_written = 0;
WriteFile(log_file, static_cast<const void*>(log.data()),
static_cast<DWORD>(log.size()), &num_written, nullptr);
+ log_file_size += num_written;
#else
- fwrite(log.data(), log.size(), 1, log_file);
+ if (fwrite(log.data(), log.size(), 1, log_file) == 1) {
Review Comment:
[replied by brpc-oncall robot] `log_file_size` is only re-synced with the
real file size when the handle is opened, and here it is not updated when
`fwrite()` fails or writes only partially (the return value is checked but
`ferror()` is not); the Windows branch has the same problem when `WriteFile`
succeeds only partially. Such a write leaves the cached size below the real
size for the rest of the process lifetime, so rotation keeps being delayed by
the unwritten amount. Consider re-reading the size with `fstat()` (or
invalidating the cache so the next call re-syncs it) when a write fails.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
