Skip to content

Commit bfb3a8e

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_SOURCELOC_STRINGS` to take ownership of the strings
1 parent ac55e60 commit bfb3a8e

File tree

3 files changed

+43
-8
lines changed

3 files changed

+43
-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: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,10 @@ option(SPDLOG_SANITIZE_ADDRESS "Enable address sanitizer in tests" OFF)
8484
# warning options
8585
option(SPDLOG_BUILD_WARNINGS "Enable compiler warnings" OFF)
8686

87+
# owning sourceloc strings options
88+
option(SPDLOG_ASYNC_OWNING_SOURCELOC_STRINGS "Async loggers take owership of source_loc strings" OFF)
89+
90+
8791
# install options
8892
option(SPDLOG_SYSTEM_INCLUDES "Include as system headers (skip for clang-tidy)." OFF)
8993
option(SPDLOG_INSTALL "Generate the install target" ${SPDLOG_MASTER_PROJECT})
@@ -248,6 +252,7 @@ foreach(
248252
SPDLOG_NO_TLS
249253
SPDLOG_NO_ATOMIC_LEVELS
250254
SPDLOG_DISABLE_DEFAULT_LOGGER
255+
SPDLOG_ASYNC_OWNING_SOURCELOC_STRINGS
251256
SPDLOG_USE_STD_FORMAT)
252257
if(${SPDLOG_OPTION})
253258
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)