6615: Fix logging (fixes #6611) r=Robyt3 a=def-

`@heinrich5991` Was the atomic important? Could we have a lock instead if so?

Seems a bit annoying since atomic can't have a shared_ptr inside

![Screenshot 2023-05-18 at 18 02 32](https://github.com/ddnet/ddnet/assets/2335377/2f8bcc57-2301-4a65-ada2-0e51f2b8c200)

## Checklist

- [x] Tested the change ingame
- [x] Provided screenshots if it is a visual change
- [ ] Tested in combination with possibly related configuration options
- [ ] Written a unit test (especially base/) or added coverage to integration test
- [ ] Considered possible null pointers and out of bounds array indexing
- [ ] Changed no physics that affect existing maps
- [ ] Tested the change with [ASan+UBSan or valgrind's memcheck](https://github.com/ddnet/ddnet/#using-addresssanitizer--undefinedbehavioursanitizer-or-valgrinds-memcheck) (optional)


Co-authored-by: Dennis Felsing <dennis@felsin9.de>
This commit is contained in:
bors[bot] 2023-05-18 20:43:30 +00:00 committed by GitHub
commit 2d5321d8b7
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 13 additions and 11 deletions

View file

@ -5,6 +5,7 @@
#include <atomic>
#include <cstdio>
#include <memory>
#if defined(CONF_FAMILY_WINDOWS)
#define WIN32_LEAN_AND_MEAN
@ -423,18 +424,19 @@ std::unique_ptr<ILogger> log_logger_windows_debugger()
}
#endif
void CFutureLogger::Set(std::unique_ptr<ILogger> &&pLogger)
void CFutureLogger::Set(std::shared_ptr<ILogger> pLogger)
{
ILogger *null = nullptr;
std::shared_ptr<ILogger> null;
m_PendingLock.lock();
ILogger *pLoggerRaw = pLogger.release();
if(!m_pLogger.compare_exchange_strong(null, pLoggerRaw, std::memory_order_acq_rel))
if(!std::atomic_compare_exchange_strong_explicit(&m_pLogger, &null, pLogger, std::memory_order_acq_rel, std::memory_order_acq_rel))
{
dbg_assert(false, "future logger has already been set and can only be set once");
}
m_pLogger = std::move(pLogger);
for(const auto &Pending : m_vPending)
{
pLoggerRaw->Log(&Pending);
m_pLogger->Log(&Pending);
}
m_vPending.clear();
m_vPending.shrink_to_fit();
@ -443,7 +445,7 @@ void CFutureLogger::Set(std::unique_ptr<ILogger> &&pLogger)
void CFutureLogger::Log(const CLogMessage *pMessage)
{
ILogger *pLogger = m_pLogger.load(std::memory_order_acquire);
auto pLogger = std::atomic_load_explicit(&m_pLogger, std::memory_order_acquire);
if(pLogger)
{
pLogger->Log(pMessage);
@ -456,7 +458,7 @@ void CFutureLogger::Log(const CLogMessage *pMessage)
void CFutureLogger::GlobalFinish()
{
ILogger *pLogger = m_pLogger.load(std::memory_order_acquire);
auto pLogger = std::atomic_load_explicit(&m_pLogger, std::memory_order_acquire);
if(pLogger)
{
pLogger->GlobalFinish();

View file

@ -202,7 +202,7 @@ std::unique_ptr<ILogger> log_logger_windows_debugger();
class CFutureLogger : public ILogger
{
private:
std::atomic<ILogger *> m_pLogger;
std::shared_ptr<ILogger> m_pLogger;
std::vector<CLogMessage> m_vPending;
std::mutex m_PendingLock;
@ -211,7 +211,7 @@ public:
* Replace the `CFutureLogger` instance with the given logger. It'll
* receive all log messages sent to the `CFutureLogger` so far.
*/
void Set(std::unique_ptr<ILogger> &&pLogger);
void Set(std::shared_ptr<ILogger> pLogger);
void Log(const CLogMessage *pMessage) override;
void GlobalFinish() override;
};

View file

@ -30,7 +30,7 @@ public:
IStorage *m_pStorage;
bool m_Logging;
std::shared_ptr<ILogger> m_pFutureLogger;
std::shared_ptr<CFutureLogger> m_pFutureLogger;
char m_aAppName[256];
@ -115,7 +115,7 @@ public:
void SetAdditionalLogger(std::shared_ptr<ILogger> &&pLogger) override
{
m_pFutureLogger = pLogger;
m_pFutureLogger->Set(pLogger);
}
};