Skip to content

Commit 387a06f

Browse files
committed
async_msg: take ownership of filename/funcname
the async logger cannot ensure that filename/funcname pointers are still valid at the time when the async worker is executed: * filename/funcname may be provided by a VM * filename/funcname may point to a readonly text field, in a shared object that had been `dlclose`d we add an api that can be enabled via `SPDLOG_ASYNC_OWNING_STRINGS` to take ownership of the strings
1 parent ac55e60 commit 387a06f

File tree

3 files changed

+41
-8
lines changed

3 files changed

+41
-8
lines changed

.github/workflows/ci.yml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@ jobs:
1818
- { compiler: gcc, version: 12, build_type: Release, cppstd: 20 }
1919
- { compiler: clang, version: 12, build_type: Debug, cppstd: 17, asan: OFF }
2020
- { compiler: clang, version: 15, build_type: Release, cppstd: 20, asan: OFF }
21+
async_owning_sourceloc_strings:
22+
- ON
23+
- OFF
2124
container:
2225
image: ${{ matrix.config.compiler == 'clang' && 'teeks99/clang-ubuntu' || matrix.config.compiler }}:${{ matrix.config.version }}
2326
name: "${{ matrix.config.compiler}} ${{ matrix.config.version }} (C++${{ matrix.config.cppstd }}, ${{ matrix.config.build_type }})"
@@ -51,6 +54,7 @@ jobs:
5154
-DSPDLOG_BUILD_BENCH=OFF \
5255
-DSPDLOG_BUILD_TESTS=ON \
5356
-DSPDLOG_BUILD_TESTS_HO=OFF \
57+
-DSPDLOG_ASYNC_OWNING_SOURCELOC_STRINGS=${{ matrix.async_owning_sourceloc_strings }} \
5458
-DSPDLOG_SANITIZE_ADDRESS=${{ matrix.config.asan || 'ON' }}
5559
make -j2
5660
ctest -j2 --output-on-failure

CMakeLists.txt

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -113,6 +113,8 @@ else()
113113
set(SPDLOG_WCHAR_FILENAMES OFF CACHE BOOL "non supported option" FORCE)
114114
endif()
115115

116+
set(SPDLOG_ASYNC_OWNING_SOURCELOC_STRINGS BOOL "Async loggers take owership of source_loc strings" OFF)
117+
116118
if(${CMAKE_SYSTEM_NAME} STREQUAL "Linux")
117119
option(SPDLOG_CLOCK_COARSE "Use CLOCK_REALTIME_COARSE instead of the regular clock," OFF)
118120
else()
@@ -248,6 +250,7 @@ foreach(
248250
SPDLOG_NO_TLS
249251
SPDLOG_NO_ATOMIC_LEVELS
250252
SPDLOG_DISABLE_DEFAULT_LOGGER
253+
SPDLOG_ASYNC_OWNING_SOURCELOC_STRINGS
251254
SPDLOG_USE_STD_FORMAT)
252255
if(${SPDLOG_OPTION})
253256
target_compile_definitions(spdlog PUBLIC ${SPDLOG_OPTION})

include/spdlog/details/thread_pool.h

Lines changed: 34 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
#include <chrono>
1111
#include <functional>
1212
#include <memory>
13+
#include <string>
1314
#include <thread>
1415
#include <vector>
1516

@@ -34,29 +35,48 @@ struct async_msg : log_msg_buffer {
3435
// should only be moved in or out of the queue..
3536
async_msg(const async_msg &) = delete;
3637

37-
// support for vs2013 move
38-
#if defined(_MSC_VER) && _MSC_VER <= 1800
3938
async_msg(async_msg &&other)
4039
: log_msg_buffer(std::move(other)),
4140
msg_type(other.msg_type),
42-
worker_ptr(std::move(other.worker_ptr)) {}
41+
worker_ptr(std::move(other.worker_ptr)) {
42+
#ifdef SPDLOG_ASYNC_OWNING_SOURCELOC_STRINGS
43+
source.filename = filename_string.c_str();
44+
source.funcname = function_string.c_str();
45+
#endif
46+
}
4347

4448
async_msg &operator=(async_msg &&other) {
4549
*static_cast<log_msg_buffer *>(this) = std::move(other);
4650
msg_type = other.msg_type;
4751
worker_ptr = std::move(other.worker_ptr);
52+
#ifdef SPDLOG_ASYNC_OWNING_SOURCELOC_STRINGS
53+
source.filename = filename_string.c_str();
54+
source.funcname = function_string.c_str();
55+
#endif
4856
return *this;
4957
}
50-
#else // (_MSC_VER) && _MSC_VER <= 1800
51-
async_msg(async_msg &&) = default;
52-
async_msg &operator=(async_msg &&) = default;
53-
#endif
5458

5559
// construct from log_msg with given type
5660
async_msg(async_logger_ptr &&worker, async_msg_type the_type, const details::log_msg &m)
5761
: log_msg_buffer{m},
5862
msg_type{the_type},
59-
worker_ptr{std::move(worker)} {}
63+
worker_ptr{std::move(worker)} {
64+
#ifdef SPDLOG_ASYNC_OWNING_SOURCELOC_STRINGS
65+
// take ownership of filename/funcname
66+
if (m.source.filename) {
67+
filename_string = std::string{m.source.filename};
68+
source.filename = filename_string.c_str();
69+
} else {
70+
source.filename = nullptr;
71+
}
72+
if (m.source.funcname) {
73+
function_string = std::string{m.source.funcname};
74+
source.funcname = function_string.c_str();
75+
} else {
76+
source.funcname = nullptr;
77+
}
78+
#endif
79+
}
6080

6181
async_msg(async_logger_ptr &&worker, async_msg_type the_type)
6282
: log_msg_buffer{},
@@ -65,6 +85,12 @@ struct async_msg : log_msg_buffer {
6585

6686
explicit async_msg(async_msg_type the_type)
6787
: async_msg{nullptr, the_type} {}
88+
89+
#ifdef SPDLOG_ASYNC_OWNING_SOURCELOC_STRINGS
90+
// source.filename/funcname always need to point to the member strings
91+
std::string filename_string;
92+
std::string function_string;
93+
#endif
6894
};
6995

7096
class SPDLOG_API thread_pool {

0 commit comments

Comments
 (0)