Skip to content

Commit 5aaebab

Browse files
LimiNodeclaude
andcommitted
fix(logger): eliminate deadlock and data race in set_config()
FileLogger and UniqueFileLogger set_config() held m_mutex while calling executor->wait(), but async tasks in those executors also acquire m_mutex to perform write_log(). This caused a deadlock when a pending task tried to lock m_mutex while set_config() was waiting for it under the same lock. Additionally, set_config() did not take m_lifecycle_mutex, so log() could read m_config/m_executor while they were being modified -> use-after-move / data race. Fix: - set_config() now takes m_lifecycle_mutex first to serialize with log(). - executor->wait() is called WITHOUT holding m_mutex, removing the deadlock. - FileLogger flushes m_file under a brief m_mutex scope after the wait. - m_config/m_executor mutations remain under m_mutex for consistency. - old_executor->shutdown() is still performed outside all locks. Scope-risk: moderate (fixes a deadlock introduced in the same PR chain) Confidence: high Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
1 parent d747734 commit 5aaebab

2 files changed

Lines changed: 40 additions & 26 deletions

File tree

include/logit_cpp/logit/loggers/FileLogger.hpp

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -211,28 +211,37 @@ namespace logit {
211211
void set_config(const Config& config) {
212212
std::unique_ptr<detail::SingleThreadExecutor> old_executor;
213213
{
214-
std::unique_lock<std::mutex> lock(m_mutex);
214+
std::lock_guard<std::mutex> lifecycle_lock(m_lifecycle_mutex);
215215
if (m_shutdown.load(std::memory_order_acquire)) return;
216+
216217
if (m_config.async) {
217218
if (m_executor) {
218219
m_executor->wait();
219220
} else {
220221
detail::TaskExecutor::get_instance().wait();
221222
}
223+
{
224+
std::lock_guard<std::mutex> lock(m_mutex);
225+
if (m_file.is_open()) m_file.flush();
226+
}
222227
}
223-
m_config.async = config.async;
224-
m_config.use_dedicated_executor = config.use_dedicated_executor;
225-
m_config.queue_capacity = config.queue_capacity;
226-
m_config.queue_policy = config.queue_policy;
227-
if (m_config.async && m_config.use_dedicated_executor) {
228-
if (!m_executor) {
229-
m_executor.reset(new detail::SingleThreadExecutor());
228+
229+
{
230+
std::lock_guard<std::mutex> lock(m_mutex);
231+
m_config.async = config.async;
232+
m_config.use_dedicated_executor = config.use_dedicated_executor;
233+
m_config.queue_capacity = config.queue_capacity;
234+
m_config.queue_policy = config.queue_policy;
235+
if (m_config.async && m_config.use_dedicated_executor) {
236+
if (!m_executor) {
237+
m_executor.reset(new detail::SingleThreadExecutor());
238+
}
239+
m_executor->set_max_queue_size(m_config.queue_capacity);
240+
m_executor->set_queue_policy(m_config.queue_policy);
241+
} else if (m_executor) {
242+
old_executor = std::move(m_executor);
243+
m_executor.reset();
230244
}
231-
m_executor->set_max_queue_size(m_config.queue_capacity);
232-
m_executor->set_queue_policy(m_config.queue_policy);
233-
} else if (m_executor) {
234-
old_executor = std::move(m_executor);
235-
m_executor.reset();
236245
}
237246
}
238247
if (old_executor) {

include/logit_cpp/logit/loggers/UniqueFileLogger.hpp

Lines changed: 18 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -158,28 +158,33 @@ namespace logit {
158158
void set_config(const Config& config) {
159159
std::unique_ptr<detail::SingleThreadExecutor> old_executor;
160160
{
161-
std::unique_lock<std::mutex> lock(m_mutex);
161+
std::lock_guard<std::mutex> lifecycle_lock(m_lifecycle_mutex);
162162
if (m_shutdown.load(std::memory_order_acquire)) return;
163+
163164
if (m_config.async) {
164165
if (m_executor) {
165166
m_executor->wait();
166167
} else {
167168
detail::TaskExecutor::get_instance().wait();
168169
}
169170
}
170-
m_config.async = config.async;
171-
m_config.use_dedicated_executor = config.use_dedicated_executor;
172-
m_config.queue_capacity = config.queue_capacity;
173-
m_config.queue_policy = config.queue_policy;
174-
if (m_config.async && m_config.use_dedicated_executor) {
175-
if (!m_executor) {
176-
m_executor.reset(new detail::SingleThreadExecutor());
171+
172+
{
173+
std::lock_guard<std::mutex> lock(m_mutex);
174+
m_config.async = config.async;
175+
m_config.use_dedicated_executor = config.use_dedicated_executor;
176+
m_config.queue_capacity = config.queue_capacity;
177+
m_config.queue_policy = config.queue_policy;
178+
if (m_config.async && m_config.use_dedicated_executor) {
179+
if (!m_executor) {
180+
m_executor.reset(new detail::SingleThreadExecutor());
181+
}
182+
m_executor->set_max_queue_size(m_config.queue_capacity);
183+
m_executor->set_queue_policy(m_config.queue_policy);
184+
} else if (m_executor) {
185+
old_executor = std::move(m_executor);
186+
m_executor.reset();
177187
}
178-
m_executor->set_max_queue_size(m_config.queue_capacity);
179-
m_executor->set_queue_policy(m_config.queue_policy);
180-
} else if (m_executor) {
181-
old_executor = std::move(m_executor);
182-
m_executor.reset();
183188
}
184189
}
185190
if (old_executor) {

0 commit comments

Comments
 (0)