Skip to content

Commit 78e2685

Browse files
committed
logger: make logger thread-safe
Current logger implementation is not thread-safe, so the commit rewrites it. Firstly, `localtime` is not thread-safe since it uses a static buffer under the hood. We don't use its result anyway (we somewhy obtain current time but don't print it), so let's simply drop it. Secondly, despite the fact `std::cout` and `std::cerr` are thread-safe according to C++11 standard, some compilers don't stick to this contract so they shouldn't be actually used from multiple threads without synchronization. Let's simply use `write` instead - it is guaranteed to be thread-safe. Part of #110
1 parent ac10240 commit 78e2685

File tree

1 file changed

+23
-13
lines changed

1 file changed

+23
-13
lines changed

src/Utils/Logger.hpp

Lines changed: 23 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -31,8 +31,9 @@
3131
*/
3232

3333
#include <time.h>
34+
#include <unistd.h>
3435

35-
#include <iostream>
36+
#include <sstream>
3637
#include <string_view>
3738

3839
enum LogLevel {
@@ -66,23 +67,32 @@ class Logger {
6667
Logger(LogLevel lvl) : m_LogLvl(lvl) {};
6768

6869
template <class... ARGS>
69-
void log(std::ostream& strm, LogLevel log_lvl,
70-
const char *file, int line, ARGS&& ...args)
70+
void log(int fd, LogLevel log_lvl, const char *file,
71+
int line, ARGS&& ...args)
7172
{
7273
if (!isLogPossible(log_lvl))
7374
return;
74-
time_t rawTime;
75-
time(&rawTime);
76-
struct tm *timeInfo = localtime(&rawTime);
77-
char timeString[10];
78-
strftime(timeString, sizeof(timeString), "%H:%M:%S", timeInfo);
79-
// The line below is commented for compatibility with previous
80-
// version. I'm not sure it was bug or feature, but the time,
81-
// filename and line was not printed.
75+
/* File and line were never printed (by mistake, I guess). */
8276
(void)file; (void)line;
83-
//strm << timeString << " " << file << ':' << line << ' ';
77+
/*
78+
* Standard streams (`cout` and `cerr`) are thread-safe
79+
* according to C++11 or more modern standards, but it turns
80+
* out that some compilers do not stick to this contract.
81+
*
82+
* Let's use `stringstream` to build a string and then write
83+
* it manually with `write` since it is guaranteed to be
84+
* thread-safe. Yes, that's slower because of unnnecessary
85+
* allocations and copies, but the log is used generally for
86+
* debug or logging exceptional cases (errors) anyway, so
87+
* that's not a big deal.
88+
*
89+
* Related: https://github.com/llvm/llvm-project/issues/51851
90+
*/
91+
std::stringstream strm;
8492
strm << log_lvl << ": ";
8593
(strm << ... << std::forward<ARGS>(args)) << '\n';
94+
std::string str = strm.str();
95+
(void)write(fd, std::data(str), std::size(str));
8696
}
8797
void setLogLevel(LogLevel lvl)
8898
{
@@ -106,7 +116,7 @@ template <class... ARGS>
106116
void
107117
log(LogLevel level, const char *file, int line, ARGS&& ...args)
108118
{
109-
gLogger.log(level == ERROR ? std::cerr : std::cout,
119+
gLogger.log(level == ERROR ? STDERR_FILENO : STDOUT_FILENO,
110120
level, file, line, std::forward<ARGS>(args)...);
111121
}
112122

0 commit comments

Comments
 (0)