Handle segfault in Ruby 2.6.6 on thread-locals

[kddnewton]

When you're trying to access a thread-local variable on Ruby < 2.7, and you call thread_variable_get on a Process::Waiter (a subclass of Thread), it will segfault. This adds a check for which Ruby version you're on to make it safe for Ruby 2.6.

Closes #2566.

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] or [ci skip] to the pull request title.
• I have added (or updated) 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.

[kddnewton]

I'm not entirely sure how to test/document this or if we even need to tbh. Let me know what you think.

[kddnewton]

@MSP-Greg tagging you since you commented on the issue

[MSP-Greg]

@kddeisz

I'm wondering if thread_variable?(:fork_safe) should be added. The behavior if the variable doesn't exist isn't clearly mentioned in the Ruby docs, it appears that the call is similar to a hash, where nil is returned if the variable/key doesn't exist. That might change?

Since @nateberkopec did the original commit, and I'm not that familiar with "Rails' reaper thread", he should also review.

[nateberkopec]

So if I understand this correctly, Process::Waiter threads are created by Process.detach.

I think we should clarify if this type of thread is always safe or is always unsafe to exist during fork. It's a pretty simple thread.

[kddnewton]

I think either way the issue is calling thread_variable_get on it for versions < 2.7. Whether or not it is always safe or not is an optimization that is kind of distinct from the bug.

[nateberkopec]

If we can avoid it, I'd rather avoid the RUBY_VERSION check. So if it's always unsafe or always safe, we can simply exclude it and never need to call thread_local_variable_get on it for any ruby version.

[kddnewton]

I think that's fine. I just have no idea how to tell if it's fork safe or not.

[nateberkopec]

OK, so the following needs to be tested:

In a Ruby process, if you Process.detach something (something will live a very long time), and then call fork afterward, that Process::Waiter thread will not be present in the fork, correct?

If true, that is definitely dangerous and could potentially break people's apps (especially in a refork Puma where the master process may get killed).

[kddnewton]

@nateberkopec I think that's correct, so the logic should be good now as it includes waiters in the list of non-fork-safe threads. I've updated to remove the ruby version check.

[nateberkopec]

Wunderbar!