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

Improve leaked thread detector #1435

Merged
merged 1 commit into from
Apr 14, 2021

Conversation

ivoanjo
Copy link
Member

@ivoanjo ivoanjo commented Mar 30, 2021

While I was chasing down a leaked thread, I gathered a few improvements. These mostly affect tests that finish too fast, my
test case was:

RSpec.describe "A short-lived thread" do
  it 'lives for a really short while' do
    Thread.new { }
  end
end

Improvements:

  • Set the @caller before the thread actually starts running. This works fine (we're using a similar construct in the profiler), and ensures that the @caller is available from the moment the thread gets created, instead of being a race.

    I observed that if a test run too fast, it was possible that @caller was not set because the thread still hadn't run by the time the detector started working.

  • Call Thread.pass whenever a backtrace is not available -- this tries to give newly-created threads a chance to run, so that we can have a backtrace for them.

  • Detect when a backtrace is missing because a thread has died.

@ivoanjo ivoanjo requested a review from a team March 30, 2021 13:30
marcotc
marcotc previously approved these changes Mar 30, 2021
While I was chasing down a leaked thread, I gathered a few
improvements. These mostly affect tests that finish too fast, my
test case was:

```ruby
RSpec.describe "A short-lived thread" do
  it 'lives for a really short while' do
    Thread.new { }
  end
end
```

Improvements:

* Set the `@caller` before the thread actually starts running. This
  works fine (we're using a similar construct in the profiler), and
  ensures that the `@caller` is available from the moment the thread
  gets created, instead of being a race.

  I observed that if a test run too fast, it was possible that
  `@caller` was not set because the thread still hadn't run by the
  time the detector started working.

* Call `Thread.pass` whenever a backtrace is not available -- this
  tries to give newly-created threads a chance to run, so that we
  can have a backtrace for them.

* Detect when a backtrace is missing because a thread has died.
@ivoanjo ivoanjo force-pushed the ivoanjo/improve-leaked-thread-detector branch from 18fbfc3 to 3edae2e Compare April 12, 2021 08:56
@ivoanjo ivoanjo merged commit cffe8e6 into master Apr 14, 2021
@ivoanjo ivoanjo deleted the ivoanjo/improve-leaked-thread-detector branch April 14, 2021 08:04
@github-actions github-actions bot added this to the 0.48.0 milestone Apr 14, 2021
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