Skip to content

Commit 65accbe

Browse files
committed
Fix thread-safety comments for log rolling/reseating methods
1 parent 99fbdc6 commit 65accbe

1 file changed

Lines changed: 19 additions & 8 deletions

File tree

include/tscore/DiagsTypes.h

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -470,9 +470,11 @@ class Diags : public DebugInterface
470470
/// path. The old file descriptor is closed.
471471
/// On failure to reopen: diags_log is unchanged; the old descriptor
472472
/// remains in use.
473-
/// @note Thread-safe. The current log is flushed and the new file is opened
474-
/// outside the internal lock; only the pointer swap is performed
475-
/// under the lock. Safe to call while concurrent logging is active.
473+
/// @note Not concurrency-safe with respect to should_roll_diagslog().
474+
/// Reads diags_log outside the internal lock before acquiring it for
475+
/// the pointer swap. Callers must ensure reseat and roll operations
476+
/// are externally serialized. Concurrent logging via print_va() is
477+
/// safe because print_va() holds the internal lock for all accesses.
476478
bool reseat_diagslog();
477479

478480
/// @brief Checks rolling conditions and rolls the diagnostics log if due.
@@ -483,8 +485,11 @@ class Diags : public DebugInterface
483485
///
484486
/// @return true if the log was rolled. false if no rolling was needed,
485487
/// or if diags_log is nullptr or uninitialized.
486-
/// @note Thread-safe. File operations (stat, flush, roll) occur outside the
487-
/// internal lock; only the pointer swap is performed under the lock.
488+
/// @note Not concurrency-safe with respect to reseat_diagslog().
489+
/// Reads diags_log outside the internal lock before acquiring it for
490+
/// the pointer swap. Callers must ensure roll and reseat operations
491+
/// are externally serialized. Concurrent logging via print_va() is
492+
/// safe because print_va() holds the internal lock for all accesses.
488493
bool should_roll_diagslog();
489494

490495
/// @brief Checks rolling conditions and rolls stdout/stderr output logs if due.
@@ -493,7 +498,10 @@ class Diags : public DebugInterface
493498
/// the same file, both pointers are updated atomically.
494499
///
495500
/// @return true if any output log was rolled. false otherwise.
496-
/// @note Thread-safe. Pointer swaps are performed under the internal lock.
501+
/// @note Pointer swaps are performed under the internal lock via
502+
/// set_std_output(). File inspection (fstat, fflush) occurs outside
503+
/// the lock. Callers must ensure no concurrent set_std_output() or
504+
/// reseat_diagslog() call is in progress.
497505
bool should_roll_outputlog();
498506

499507
/// @brief Redirects stdout or stderr to the specified file.
@@ -511,8 +519,11 @@ class Diags : public DebugInterface
511519
/// and the corresponding log pointer (stdout_log or stderr_log) is
512520
/// updated to the new handle.
513521
/// On failure: the log pointer is nullptr and the redirect did not occur.
514-
/// @note Thread-safe. The pointer swap and dup2() call are performed under
515-
/// the internal lock.
522+
/// @note The pointer swap and dup2() call are performed under the internal
523+
/// lock. However, the old pointer snapshot is read outside the lock,
524+
/// so this method is not concurrency-safe with respect to concurrent
525+
/// calls to itself or should_roll_outputlog(). Callers must ensure
526+
/// these operations are externally serialized.
516527
bool set_std_output(StdStream stream, const char *file);
517528

518529
/// The debug tag pattern passed at construction time. May be nullptr if no

0 commit comments

Comments
 (0)