Skip to content

fix: do not stop sampling if Some(None) stacktrace is observed #396

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

Closed
wants to merge 1 commit into from

Conversation

korniltsev
Copy link
Contributor

No description provided.

@acj
Copy link
Member

acj commented Jun 2, 2025

Seems like a reasonable change, but I'd like to understand how you ran into it. Does this only happen if on_cpu_only is true? Can you give any details or repro steps?

@korniltsev
Copy link
Contributor Author

I found this in pyroscope where rbspy is used as a library. There it manifests as just no stacktraces to the user of the library. There I start a puma server, it becomes idle (no requests), all samplers are exiting due to incorrect break, later when puma is not idle, no stacktraces is sent.

@korniltsev
Copy link
Contributor Author

I guess I can reproduce with the rbspy binary as well

sudo ./target/debug/rbspy record --pid=642619 -s -c

before

Time since start: 1s. Press Ctrl+C to stop.
Summary of profiling data so far:
% self  % total  name
Warning: no profile samples were collected
Wrote raw data to /root/.cache/rbspy/2025-06-03-iIvBIWeeW0.raw.gz
Wrote formatted output to /root/.cache/rbspy/2025-06-03-onWD4497lK.flamegraph.svg

after:

Time since start: 10s. Press Ctrl+C to stop.
Summary of profiling data so far:
% self  % total  name
100.00   100.00  fibonacci - /home/korniltsev/p/ruby_app/app.rb:17
...

@korniltsev
Copy link
Contributor Author

Does this only happen if on_cpu_only is true

I have not tried on_cpu_only=false

Reading the code i suspect only oncpu is affected

@korniltsev
Copy link
Contributor Author

Out of curiosity: is there a reason to use continue and skip the sleeping ?

acj added a commit that referenced this pull request Jun 11, 2025
Ref: #396

Co-authored-by: Tolya Korniltsev <korniltsev.anatoly@gmail.com>
@acj
Copy link
Member

acj commented Jun 11, 2025

No, that was my mistake. I spent some time trying to develop a test for this scenario and misremembered where the sleep code was. Thanks for asking 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants