Skip to content
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

Use SIGWINCH to print thread backtraces #2195

Merged
merged 1 commit into from Mar 24, 2020

Conversation

stanhu
Copy link
Contributor

@stanhu stanhu commented Mar 22, 2020

Description

#1320 added support for using SIGINFO,
but this is only available on BSD-based systems. SIGWINCH is on Linux.

Your checklist for this pull request

  • I have reviewed the guidelines for contributing to this repository.
  • I have added an entry to History.md if this PR fixes a bug or adds a feature. If it doesn't need an entry to HISTORY.md, I have added [changelog skip] the pull request title.
  • I have added appropriate tests if this PR fixes a bug or adds a feature.
  • My pull request is 100 lines added/removed or less so that it can be easily reviewed.
  • If this PR doesn't need tests (docs change), I added [ci skip] to the title of the PR.
  • If this closes any issues, I have added "Closes #issue" to the PR description or my commit messages.
  • I have updated the documentation accordingly.
  • All new and existing tests passed, including Rubocop.

@stanhu stanhu force-pushed the sh-use-siginfo-backtrace branch 3 times, most recently from 3e20183 to 4a33fad Compare March 22, 2020 15:12
@nateberkopec
Copy link
Member

SIGPROF is already used pretty widely in profilers, e.g. stackprof.

I'm not sure we can use that signal.

@nateberkopec
Copy link
Member

Just looking at the BSD signal list, maybe SIGVTALRM or SIGWINCH? I'd like to know what people get for the Signal.list on other platforms, btw.

@nateberkopec
Copy link
Member

NVM, Ruby uses SIGVTALRM internally. SIGWINCH however probably won't interfere with anyone's existing handlers in a webapp.

@stanhu
Copy link
Contributor Author

stanhu commented Mar 24, 2020

On Linux, I get:

{"EXIT"=>0, "HUP"=>1, "INT"=>2, "QUIT"=>3, "ILL"=>4, "TRAP"=>5, "ABRT"=>6, "IOT"=>6, "FPE"=>8, "KILL"=>9, "BUS"=>7, "SEGV"=>11, "SYS"=>31, "PIPE"=>13, "ALRM"=>14, "TERM"=>15, "URG"=>23, "STOP"=>19, "TSTP"=>20, "CONT"=>18, "CHLD"=>17, "CLD"=>17, "TTIN"=>21, "TTOU"=>22, "IO"=>29, "XCPU"=>24, "XFSZ"=>25, "VTALRM"=>26, "PROF"=>27, "WINCH"=>28, "USR1"=>10, "USR2"=>12, "PWR"=>30, "POLL"=>29}

@nateberkopec
Copy link
Member

Sounds like SIGWINCH is probably the best option then, yeh?

puma#1320 added support for using SIGINFO,
but this is only available on BSD-based systems. SIGWINCH is on Linux.

This is useful for debugging infinite loops or slow performance.
@stanhu stanhu changed the title Use SIGPROF to print thread backtraces Use SIGWINCH to print thread backtraces Mar 24, 2020
@stanhu
Copy link
Contributor Author

stanhu commented Mar 24, 2020

Yes, thanks.

I've also wondered why attaching gdb to Puma and calling call rb_backtrace() doesn't work like it does in Unicorn and Sidekiq.

@nateberkopec nateberkopec merged commit 3bf45fb into puma:master Mar 24, 2020
@nateberkopec
Copy link
Member

I've also wondered why attaching gdb to Puma and calling call rb_backtrace() doesn't work like it does in Unicorn and Sidekiq.

Can you open a new issue?

@nateberkopec
Copy link
Member

I think we're going to have to revert. SIGWINCH gets sent all the time when changing window sizes and so this makes terminal output kind of unusable locally.

I'm not sure any other signal would work.

@stanhu
Copy link
Contributor Author

stanhu commented Mar 25, 2020

Ok. Is it possible to use TTIN as Sidekiq does, and then make it clear that signal does two different things, depending on whether it's sent to the worker process or the cluster worker?

@nateberkopec
Copy link
Member

Maybe. Do we listen for TTIN in child workers at all right now?

@stanhu
Copy link
Contributor Author

stanhu commented Mar 25, 2020

@nateberkopec Not at the moment. I think I got it to work. It would be a breaking change for anyone issuing TTIN on the cluster workers, though.

@eregon
Copy link
Contributor

eregon commented Apr 2, 2020

FWIW, TruffleRuby prints all thread backtraces on SIGALRM.
Maybe we should ask for this feature in MRI instead of adding it here?

@dentarg
Copy link
Member

dentarg commented Apr 2, 2020

@eregon Nate commented "Ruby uses SIGVTALRM internally" above, but I'm not sure for what (from a really quick look)

@eregon
Copy link
Contributor

eregon commented Apr 2, 2020

SIGALRM is different than SIGVTALRM (which is reserved in MRI & TruffleRuby).
SIGVTALRM is used to interrupt blocking system calls in MRI & TruffleRuby.

See https://github.com/ruby/spec/blob/ebdf0d1b06b39374a85baaae53771449ab1fe3d7/core/signal/trap_spec.rb#L118-L133 for the list of reserved signals for each Ruby implementation.

@dentarg
Copy link
Member

dentarg commented Apr 2, 2020

Thanks for clarifying that @eregon

(I should have searched for SIGVTALRM not just ALRM, the GitHub search doesn't seem to surface partial hits)

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

Successfully merging this pull request may close these issues.

None yet

4 participants