it leaks #100
Comments
|
tests leak too. |
|
it fails with fmtlib 11.0.2 , 10.2.1 and 9.1.0 . It seems to be definitely fmtlog's problem. |
|
It seems that in
So, at least here's two no-no:
I'll try change the std::string_view to std::string. |
|
Just fucking nice: one place fixed, got one test fixed and other tests leak in some other places. I'm loving it (no) |
|
just... why ... why it's in C's char pointer and not it std::string or any like |
|
Just amazing: solved another leak via converting a lot of dangling fmt::string_view to std::string... Creating thread buffer in heap and then saving pointer in std::vector<type *> ...... clear does not work if the type don't have appropriate destructor... |
|
Memory management in fmtlog is a little bit tricky, which may trigger false alarm by sanitizers, so don't care too much about it. |
IDK , I solved them somehow, but now fmtlog requires |
|
Thinking leak sanitizer would not understand library logic and trigger a false positive is a bold statement. Same leak on valgrind: log_test: Who want a 1MB leak in their app? Anyway, here is my attempt to at least fix the leaks. I saw there was some logic to clean everything up, but it doesn't always work. diff --git a/fmt b/fmt
--- a/fmt
+++ b/fmt
@@ -1 +1 @@
-Subproject commit e69e5f977d458f2650bb346dadf2ad30c5320281
+Subproject commit e69e5f977d458f2650bb346dadf2ad30c5320281-dirty
diff --git a/fmtlog-inl.h b/fmtlog-inl.h
index f02cab6..bc8dadd 100644
--- a/fmtlog-inl.h
+++ b/fmtlog-inl.h
@@ -103,10 +103,10 @@ public:
setHeaderPattern("{HMSf} {s:<16} {l}[{t:<6}] ");
logInfos.reserve(32);
bgLogInfos.reserve(128);
- bgLogInfos.emplace_back(nullptr, nullptr, fmtlog::DBG, fmt::string_view());
- bgLogInfos.emplace_back(nullptr, nullptr, fmtlog::INF, fmt::string_view());
- bgLogInfos.emplace_back(nullptr, nullptr, fmtlog::WRN, fmt::string_view());
- bgLogInfos.emplace_back(nullptr, nullptr, fmtlog::ERR, fmt::string_view());
+ bgLogInfos.emplace_back(nullptr, nullptr, fmtlog::DBG, std::string{});
+ bgLogInfos.emplace_back(nullptr, nullptr, fmtlog::INF, std::string{});
+ bgLogInfos.emplace_back(nullptr, nullptr, fmtlog::WRN, std::string{});
+ bgLogInfos.emplace_back(nullptr, nullptr, fmtlog::ERR, std::string{});
threadBuffers.reserve(8);
bgThreadBuffers.reserve(8);
memset(membuf.data(), 0, membuf.capacity());
@@ -119,7 +119,6 @@ public:
}
void setHeaderPattern(const char* pattern) {
- if (shouldDeallocateHeader) delete[] headerPattern.data();
using namespace fmt::literals;
for (int i = 0; i < parttenArgSize; i++) {
reorderIdx[i] = parttenArgSize - 1;
@@ -130,7 +129,6 @@ public:
"l"_a = fmtlog::LogLevel(), "s"_a = "fmtlog.cc:123", "g"_a = "/home/raomeng/fmtlog/fmtlog.cc:123", "Ymd"_a = "",
"HMS"_a = "", "HMSe"_a = "", "HMSf"_a = "", "HMSF"_a = "", "YmdHMS"_a = "", "YmdHMSe"_a = "", "YmdHMSf"_a = "",
"YmdHMSF"_a = "");
- shouldDeallocateHeader = headerPattern.data() != pattern;
setArg<0>(fmt::string_view(weekdayName.s, 3));
setArg<1>(fmt::string_view(monthName.s, 3));
@@ -177,7 +175,7 @@ public:
struct StaticLogInfo
{
// Constructor
- constexpr StaticLogInfo(fmtlog::FormatToFn fn, const char* loc, fmtlog::LogLevel level, fmt::string_view fmtString)
+ StaticLogInfo(fmtlog::FormatToFn fn, const char* loc, fmtlog::LogLevel level, std::string fmtString)
: formatToFn(fn)
, formatString(fmtString)
, location(loc)
@@ -207,7 +205,7 @@ public:
inline fmt::string_view getLocation() { return fmt::string_view(location, endPos); }
fmtlog::FormatToFn formatToFn;
- fmt::string_view formatString;
+ std::string formatString;
const char* location;
uint8_t basePos;
uint8_t endPos;
@@ -217,8 +215,7 @@ public:
static thread_local ThreadBufferDestroyer sbc;
int64_t midnightNs;
- fmt::string_view headerPattern;
- bool shouldDeallocateHeader = false;
+ std::string headerPattern;
FILE* outputFp = nullptr;
bool manageFp = false;
size_t fpos = 0; // file position of membuf, used only when manageFp == true
@@ -227,13 +224,13 @@ public:
uint32_t flushBufSize = 8 * 1024;
fmtlog::LogLevel flushLogLevel = fmtlog::OFF;
std::mutex bufferMutex;
- std::vector<fmtlog::ThreadBuffer*> threadBuffers;
+ std::vector<std::shared_ptr<fmtlog::ThreadBuffer>> threadBuffers;
struct HeapNode
{
- HeapNode(fmtlog::ThreadBuffer* buffer)
- : tb(buffer) {}
+ HeapNode(std::shared_ptr<fmtlog::ThreadBuffer> buffer)
+ : tb(std::move(buffer)) {}
- fmtlog::ThreadBuffer* tb;
+ std::shared_ptr<fmtlog::ThreadBuffer> tb;
const fmtlog::SPSCVarQueueOPT::MsgHeader* header = nullptr;
};
std::vector<HeapNode> bgThreadBuffers;
@@ -287,7 +284,7 @@ public:
void preallocate() {
if (fmtlog::threadBuffer) return;
- fmtlog::threadBuffer = new fmtlog::ThreadBuffer();
+ fmtlog::threadBuffer = std::make_shared<fmtlog::ThreadBuffer>();
#ifdef _WIN32
uint32_t tid = static_cast<uint32_t>(::GetCurrentThreadId());
#else
@@ -443,7 +440,6 @@ public:
if (node.header) continue;
node.header = node.tb->varq.front();
if (!node.header && node.tb->shouldDeallocate) {
- delete node.tb;
node = bgThreadBuffers.back();
bgThreadBuffers.pop_back();
i--;
@@ -494,7 +490,7 @@ fmtlogDetailT<> fmtlogDetailWrapper<_>::impl;
template<int _>
void fmtlogT<_>::registerLogInfo(uint32_t& logId, FormatToFn fn, const char* location,
- LogLevel level, fmt::string_view fmtString) noexcept {
+ LogLevel level, std::string fmtString) noexcept {
auto& d = fmtlogDetailWrapper<>::impl;
std::lock_guard<std::mutex> lock(d.logInfoMutex);
if (logId) return;
diff --git a/fmtlog.h b/fmtlog.h
index 566b34f..0061ec4 100644
--- a/fmtlog.h
+++ b/fmtlog.h
@@ -38,7 +38,7 @@ SOFTWARE.
#ifdef _WIN32
#define FAST_THREAD_LOCAL thread_local
#else
-#define FAST_THREAD_LOCAL __thread
+#define FAST_THREAD_LOCAL thread_local
#endif
// define FMTLOG_BLOCK=1 if log statment should be blocked when queue is full, instead of discarding the msg
@@ -378,7 +378,7 @@ public:
int& argIdx, std::vector<fmt::basic_format_arg<Context>>& args);
static void registerLogInfo(uint32_t& logId, FormatToFn fn, const char* location, LogLevel level,
- fmt::string_view fmtString) noexcept;
+ std::string fmtString) noexcept;
static void vformat_to(MemoryBuffer& out, fmt::string_view fmt, fmt::format_args args);
@@ -391,7 +391,7 @@ public:
TSCNS tscns;
volatile LogLevel currentLogLevel;
- static FAST_THREAD_LOCAL ThreadBuffer* threadBuffer;
+ static FAST_THREAD_LOCAL std::shared_ptr<ThreadBuffer> threadBuffer;
template<typename Arg>
static inline constexpr bool isNamedArg() {
@@ -609,19 +609,19 @@ public:
}
template<bool Reorder, typename... Args>
- static fmt::string_view unNameFormat(fmt::string_view in, uint32_t* reorderIdx,
+ static std::string unNameFormat(fmt::string_view in, uint32_t* reorderIdx,
const Args&... args) {
constexpr size_t num_named_args = fmt::detail::count<isNamedArg<Args>()...>();
if constexpr (num_named_args == 0) {
- return in;
+ return std::string{in.data(), in.size()}; // :(
}
const char* begin = in.data();
const char* p = begin;
- std::unique_ptr<char[]> unnamed_str(new char[in.size() + 1 + num_named_args * 5]);
+ std::string unnamed_str(in.size() + 1 + num_named_args * 5, '\0');
fmt::detail::named_arg_info<char> named_args[std::max(num_named_args, (size_t)1)];
storeNamedArgs<0, 0>(named_args, args...);
- char* out = (char*)unnamed_str.get();
+ char* out = unnamed_str.data();
uint8_t arg_idx = 0;
while (true) {
auto c = *p++;
@@ -663,8 +663,10 @@ public:
}
begin = p;
}
- const char* ptr = unnamed_str.release();
- return fmt::string_view(ptr, out - ptr);
+
+ unnamed_str.resize(out - unnamed_str.data());
+
+ return unnamed_str;
}
public:
@@ -723,7 +725,7 @@ public:
using fmtlog = fmtlogT<>;
template<int _>
-FAST_THREAD_LOCAL typename fmtlogT<_>::ThreadBuffer* fmtlogT<_>::threadBuffer;
+FAST_THREAD_LOCAL typename std::shared_ptr<typename fmtlogT<_>::ThreadBuffer> fmtlogT<_>::threadBuffer;
template<int __ = 0>
struct fmtlogWrapper |
|
And I don't know what to say about |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
It has a memory leak.
I'm using updated (no) version of the lib with updated fmtlib. I believe it's fmtlog issue but I don't have time to investigate it
After
logdor any like macro.The text was updated successfully, but these errors were encountered: