Skip to content

Flush sinks on program exit with a special method Sink::flush_on_exit #102

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Nov 3, 2025
Merged

Flush sinks on program exit with a special method Sink::flush_on_exit #102

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
45128ef
refactor: call flush_atexit at exit
tisonkun Oct 3, 2025
ce175c2
Update spdlog/src/sink/async_sink/async_pool_sink.rs
tisonkun Oct 3, 2025
3e52f81
forward flush_atexit to DedupSink
tisonkun Oct 3, 2025
6cb4df9
fixup
tisonkun Oct 3, 2025
baa32e9
simplify code
tisonkun Oct 4, 2025
e37ae9f
carry out clippy workaround
tisonkun Oct 4, 2025
1e4304a
fixup
tisonkun Oct 4, 2025
fb89450
rename flush_atexit to flush_on_exit
tisonkun Nov 3, 2025
4c62e5f
More detailed documentation for `Sink::flush_on_exit`
SpriteOvO Nov 3, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions spdlog/src/lib.rs
View file
Open in desktop
Copy link
Contributor Author

@tisonkun tisonkun Oct 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We currently set atexit and only hook_panic if atexit fails.

However, it seems that atexit is only called when the process is normally terminated. Not sure if a panic causes a normal exit, or an abnormal exit.

At least when panic = "abort", the atexit glue may not be called.

Copy link
Owner

@SpriteOvO SpriteOvO Oct 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not at home these days, will take a look at it days later.

Copy link
Contributor Author

@tisonkun tisonkun Oct 7, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're not in a rush. Enjoy your days :D

Copy link
Owner

@SpriteOvO SpriteOvO Oct 16, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We currently set atexit and only hook_panic if atexit fails.

However, it seems that atexit is only called when the process is normally terminated. Not sure if a panic causes a normal exit, or an abnormal exit.

At least when panic = "abort", the atexit glue may not be called.

For "unwind" panic, atexit callbacks will be called. But I didn't realize "abort" panic before, and indeed as you said, atexit doesn't work for it.

I'm not sure yet what the best solution for it is, maybe always hook panic if #[cfg(panic = "abort")]? Because we don't need the callback to be called twice (once from atexit and once from the panic handler). Anyway, I think this should be implemented in a new PR, not in this one. But it's valuable that you mentioned it, so flush_atexit is really not a good name, if the panic handler is actually calling it.

I think flush_on_exit is fine. Also, I've been thinking about the need to add an exit reason parameter:

pub enum ExitReason {
    AtExit,
    Panic,
    // Maybe added in future
    Signal(...)
}

fn flush_on_exit(&self, reason: ExitReason) { ... }

One reason is that it may be possible in the future, we call flush when a crash signal occurs (we had an attempt in PR #18), and there are stricter safety requirements (signal-safety) for exception signals, in short, many APIs can't be used in some signal handlers.

Not sure if such a parameter is necessary to add, and to add it now.

Copy link
Contributor Author

@tisonkun tisonkun Oct 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your update. I'll give this PR another round early next month.

Original file line number Diff line number Diff line change
Expand Up @@ -736,18 +736,15 @@ pub fn log_crate_proxy() -> &'static LogCrateProxy {
&PROXY
}

static IS_TEARING_DOWN: AtomicBool = AtomicBool::new(false);

fn flush_default_logger_at_exit() {
// Rust never calls `drop` for static variables.
//
// Setting up an exit handler gives us a chance to flush the default logger
// once at the program exit, thus we don't lose the last logs.

extern "C" fn handler() {
IS_TEARING_DOWN.store(true, Ordering::SeqCst);
if let Some(default_logger) = DEFAULT_LOGGER.get() {
default_logger.load().flush()
default_logger.load().flush_sinks_atexit()
}
}

Expand Down
8 changes: 8 additions & 0 deletions spdlog/src/logger.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,14 @@ impl Logger {
})
}

pub(crate) fn flush_sinks_atexit(&self) {
self.sinks.iter().for_each(|sink| {
if let Err(err) = sink.flush_atexit() {
self.handle_error(err);
}
});
}

Copy link
Contributor Author

@tisonkun tisonkun Oct 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may be even pub if users set their own global static LOGGER. But given that is not quite possible it's OK to leave pub(crate) until real use cases occur.

// This will lose the periodic flush property, if any.
#[must_use]
fn clone_lossy(&self) -> Self {
Expand Down
30 changes: 15 additions & 15 deletions spdlog/src/sink/async_sink/async_pool_sink.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,21 +129,21 @@ impl Sink for AsyncPoolSink {
}

fn flush(&self) -> Result<()> {
if crate::IS_TEARING_DOWN.load(Ordering::SeqCst) {
// https://github.com/SpriteOvO/spdlog-rs/issues/64
//
// If the program is tearing down, this will be the final flush. `crossbeam`
// uses thread-local internally, which is not supported in `atexit` callback.
// This can be bypassed by flushing sinks directly on the current thread, but
// before we do that we have to destroy the thread pool to ensure that any
// pending log tasks are completed.
self.thread_pool.destroy();
self.backend.flush()
} else {
self.assign_task(Task::Flush {
backend: self.clone_backend(),
})
}
self.assign_task(Task::Flush {
backend: self.clone_backend(),
})
}

fn flush_atexit(&self) -> Result<()> {
// https://github.com/SpriteOvO/spdlog-rs/issues/64
//
// If the program is tearing down, this will be the final flush. `crossbeam`
// uses thread-local internally, which is not supported in `atexit` callback.
// This can be bypassed by flushing sinks directly on the current thread, but
// before we do that we have to destroy the thread pool to ensure that any
// pending log tasks are completed.
self.thread_pool.destroy();
self.backend.flush()
}
}

Expand Down
7 changes: 7 additions & 0 deletions spdlog/src/sink/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,13 @@ pub trait Sink: SinkPropAccess + Sync + Send {

/// Flushes any buffered records.
fn flush(&self) -> Result<()>;

/// Flushes any buffered records at program exit.
///
/// Default to call `flush()`.
fn flush_atexit(&self) -> Result<()> {
self.flush()
}
}

/// Container type for [`Sink`]s.
Expand Down
Loading