Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

source_loc corrupted when use in async logger #3227

Closed
Ducatel opened this issue Oct 25, 2024 · 5 comments
Closed

source_loc corrupted when use in async logger #3227

Ducatel opened this issue Oct 25, 2024 · 5 comments

Comments

@Ducatel
Copy link

Ducatel commented Oct 25, 2024

Hi,

I face an issue when I use async logger +rotating_file sink + source_loc.
The file name and the function name are corrupted.

I go something like this in my output file

ùùùùùùùùùùùùùùùùùùùùùùùùùùùùù(49):µµµµµµµµµµµµµµµµµµ [unittest][critical](Thread: 20264) 2024/10/25 16:53:39.413 Log Message: a log entry -- Context:

Instead of the follwing when I'm using synchronous logger

E:\sources\libs\UnitTest\TestLog.cpp(49):UnitTest::TestLog::TestBasicUsage [unittest][critical](Thread: 20264) 2024/10/25 16:53:39.413 Log Message: a log entry -- Context:

Basically my code do

MyLogger logger; //logger created with rotating file sink
logger.writeLog(L"a log entry", __FUNCTION__, __FILE__, __LINE__, L"", MyLog::LogLevel::kCritical);
...

//// In the  MyLogger class
void MyLogger::writeLog(const std::wstring_view &strLogMessage, const std::string&strFunctionName, const std::string& strFileName, const int nLine, const std::wstring_view&strContext, LogLevel logLevel)
{
    spdlog::source_loc sourceLoc(strFileName.c_str(), nLine, strFunctionName.c_str());
    mLoggerPtr->log(sourceLoc, static_cast<spdlog::level::level_enum>(logLevel), std::format(L"Log Message: {} -- Context:{}", strLogMessage, strContext).c_str());
}

Seems really look like memory corruption when thread come into the game.

So I did a really basic test by changing the source_loc struct by

struct source_loc {
    SPDLOG_CONSTEXPR source_loc() = default;
    SPDLOG_CONSTEXPR source_loc(const char *filename_in, int line_in, const char *funcname_in)
        : filename{filename_in},
          line{line_in},
          funcname{funcname_in} {}

    SPDLOG_CONSTEXPR source_loc(const source_loc& other)
        : line(other.line) {
        if (other.filename) {
            filename = new char[std::strlen(other.filename) + 1];
            std::strcpy(const_cast<char*>(filename), other.filename);
        }
        if (other.funcname) {
            funcname = new char[std::strlen(other.funcname) + 1];
            std::strcpy(const_cast<char*>(funcname), other.funcname);
        }
    }


    SPDLOG_CONSTEXPR ~source_loc() {
        delete[] filename;
        delete[] funcname;
    }


    SPDLOG_CONSTEXPR     source_loc& operator=(const source_loc& other) {
        if (this != &other) { 
            delete[] filename;
            delete[] funcname;


            line = other.line;

            if (other.filename) {
                filename = new char[std::strlen(other.filename) + 1];
                std::strcpy(const_cast<char*>(filename), other.filename);
            }
            else {
                filename = nullptr;
            }
            if (other.funcname) {
                funcname = new char[std::strlen(other.funcname) + 1];
                std::strcpy(const_cast<char*>(funcname), other.funcname);
            }
            else {
                funcname = nullptr;
            }
        }
        return *this;
    }

    SPDLOG_CONSTEXPR bool empty() const SPDLOG_NOEXCEPT { return line <= 0; }
    const char *filename{nullptr};
    int line{0};
    const char *funcname{nullptr};
};

And it's working.
So did I do something wrong ?
Or there is a bug here ?

Thanks in advance for your help

Extra info:

  • version 1.14.1
  • Compiled with Visual studio 2022 windows x64 c++20 enabled
@gabime
Copy link
Owner

gabime commented Oct 25, 2024

The filename pointer is not deep copied by the async logger, so it seems the pointer gets invalidated by the time it get logged. You should use the original FILE pointer or retain pointer somehow.

@tt4g
Copy link
Contributor

tt4g commented Oct 25, 2024

Duplicate #3195, #3073, #2867

PR #2935

@Ducatel
Copy link
Author

Ducatel commented Oct 29, 2024

@tt4g Thanks for the info.
Why the PR wasn't merged ? There is an issue with this solution ?
If it's just because the creator doesn't reply anymore, I can do the PR myself

@tt4g
Copy link
Contributor

tt4g commented Oct 29, 2024

See the following comment (it is the decision of maintainer):

#2867 (comment)

I prefer not to add complexity to the codebase for such a rare feature.

@Ducatel
Copy link
Author

Ducatel commented Oct 29, 2024

Arf, ok, so I will but that inside the message body directly and I willl not have issue.
Thanks ;)

@Ducatel Ducatel closed this as completed Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants