Skip to content

fix: making invocation of tracing functions from Drop impls optional #238

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.

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

fogti
Copy link
Member

@fogti fogti commented Apr 29, 2025

They are now disabled by default and can be enabled using the feature flag tracing_in_drop if necessary.

Closes #231.

They are now disabled by default and can be enabled using the feature flag
`tracing_in_drop` if necessary.

Closes #231.
@taiki-e
Copy link
Collaborator

taiki-e commented May 5, 2025

It is not clear if it is sufficient to disable it in the destructor of our type, since the problem appears to be happening in the destructor of thread-local storage.

Looking at the backtrace of #231 again, it appears that it is the tracing used in the polling::Poller::delete called in the destructor of calloop::sources::generic::Generic that is actually triggering the problem.

  20: tracing::span::Span::new
             at /home/sc/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/tracing-0.1.41/src/span.rs:437:9
  21: polling::epoll::Poller::delete
             at /home/sc/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/polling-3.7.4/src/epoll.rs:145:20
  22: polling::Poller::delete
             at /home/sc/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/polling-3.7.4/src/lib.rs:689:9
 23: <calloop::sources::generic::Generic<F,E> as core::ops::drop::Drop>::drop
             at /home/sc/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/calloop-0.13.0/src/sources/generic.rs:254:13

So, I think it would be more reliable to simply make tracing optional.

@fogti
Copy link
Member Author

fogti commented May 5, 2025

This does both.

@fogti fogti force-pushed the drop-opt-tracing branch from 0e63998 to cc53eae Compare May 5, 2025 17:57
@fogti fogti force-pushed the drop-opt-tracing branch from cc53eae to cd4bbbc Compare May 5, 2025 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Use of tracing in Drop will cause SIGABRT in thread-local context
2 participants