I've a private static method in a class that has all the methods as static. It's a helper class that helps with logging and other stuff. This helper class would be called by multiple threads. I don't understand how the method in question is working safely with multiple threads without a lock.
//Helper.cpp std::recursive_mutex Logger::logMutex_; void Logger::write(const std::string &logFilePath, const std::string &formattedLog) { std::lock_guard<std::mutex> guard(logMutex_); std::ofstream logFile(logFilePath.c_str(), std::ios::out | std::ios::app); if (logFile.is_open()) { logFile << formattedLog; logFile.close(); } } void Logger::error(const string &appId, const std::string &fmt, ...) { auto logFile = validateLogFile(appId); //Also a private static method that validates if a file exists for this app ID. if (!logFile.empty()) { //Format the log write(logFile, log); } } //Helper.h class Logger { public: static void error(const std::string &Id, const std::string &fmt, ...); private: static void write(const std::string &fileName, const std::string &formattedLog); static std::recursive_mutex logMutex_; }; I understand that the local variables inside the static methods are purely local. i.e., A stack is created every time these methods are called and the variables are initialized in them. Now, in the Logger::write method I'm opening a file and writing to it. So when multiple threads calls the write method through the Logger::error method(Which is again static) and when there's no lock, I believe I should see some data race/crash.
Because multiple threads are trying to open the same file. Even if the kernel allows to open a file multiple times, I must see some fault in the data written to the file.
I tested this with running up to 100 threads and I see no crash, all the data is being written to the file concurrently. I can't completely understand how this is working. With or without the lock, I see the data is being written to the file perfectly.
TEST_F(GivenALogger, WhenLoggerMethodsAreCalledFromMultipleThreads_AllTheLogsMustBeLogged) { std::vector<std::thread> threads; int num_threads = 100; int i = 0; for (; i < num_threads / 2; i++) { threads.push_back(std::thread(&Logger::error, validId, "Trial %d", i)); } for (; i < num_threads; i++) { threads.push_back(std::thread(&Logger::debug, validId, "Trial %d", i)); } std::for_each(threads.begin(), threads.end(), [](std::thread &t) { t.join(); }); auto actualLog = getActualLog(); // Returns a vector of log lines. EXPECT_EQ(num_threads, actualLog.size()); } Also, how should I properly/safely access the file?
errordoes anything with the underlying file is through its own call towrite, which means that every write in your code actually is protected by lockinglogMutex_. It's possible that there's some undefined behavior due tovalidateLogFile, but you haven't shown the contents of that function. Second: Undefined behavior is undefined. "I believe I should see a crash" is unfounded.validateLogFilejust verifies if the file exists in the directory and returns the filePath if exists, if not an empty string.