Rewrite connection reaper test with timeout

[fatkodima]

Trying to debug reaper_test.rb.

[fatkodima]

After the second attempt (#50037), I was finally be able to debug that flaky test that is cancelled after the 30m timeout time!

See https://buildkite.com/rails/rails/builds/104764#018d9373-57da-4bb8-a019-e1e2a9c12f41/1156-1587

We fork a process in

rails/activerecord/test/cases/reaper_test.rb

Line 131 in 0f9aaa5

pid = fork do

but it never gets run and the runtime is blocked waiting for its termination

rails/activerecord/test/cases/reaper_test.rb

Line 145 in 0f9aaa5

Process.waitpid(pid)

We can wrap that waitpid with Timeout.timeout, but it will be better to understand why the process did not run? Do buildkite have a limit on some number of processes and we had leaked processes from somewhere else? Or what can be the reason?

cc @byroot Do you have any ideas?

[byroot]

but it never gets run and the runtime is blocked waiting for its termination

If the parent process is blocked on wait, it means the child process is still alive. So in a way, it "gets run".

It likely run into some sort of deadlock.

[fatkodima]

But there is no printing in child from https://github.com/rails/rails/pull/51038/files#diff-db41b8a333b0b2a49f0b5cd5361a65426419bea8746069f52f07d0ab80f359f8R149. What can be the reason?

[matthewd]

$stdout.buffer?

[byroot]

What can be the reason?

This test is about restarting the reaper thread in the child, which we do from an after_fork callback that is executed before everything else. So it probably is deadlocked in there.

rails/activesupport/lib/active_support/fork_tracker.rb

Lines 19 to 25 in a8d6d47

	def after_fork_callback
	new_pid = Process.pid
	if @pid != new_pid
	@callbacks.each(&:call)
	@pid = new_pid
	end
	end

[fatkodima]

@byroot Do you have an idea on why that can be and how to solve it? This test is so annoying ... If this is not easily solvable/debuggable, maybe we can wrap it into some timeout to reduce the damage, until it gets solved?

[byroot]

Do you have an idea on why that can be and how to solve it?

No, I'd need to dig into it. I barely had a look.

maybe we can wrap it into some timeout to reduce the damage, until it gets solved?

It's probably a good idea in general when forking to then invoke code. Might be worth extracting some sort of test helper.

The proper way to timeout a forked process being to use a pipe, allowing to do reader_pipe.wait_readable(timeout).

[fatkodima]

@byroot Rewrote that test with your suggestion. Please take a look.

CI is red, but seems unrelated.

[byroot]

You should call Process.wait even after a SIGKILL otherwise the child will linger around as a zombie.

[byroot]

Also, maybe excessive, but rather than SIGKILL, you could send SIGABRT and cause the child process to print a Ruby crash report, which would include its main thread backtrace, giving more information on where it's stuck.

[byroot]

Suggested change

	if completed
	Process.waitpid(pid)
	assert_predicate $?, :success?
	else
	Process.kill("KILL", pid)
	flunk("Process timed out")
	unless completed
	Process.kill("KILL", pid)
	end
	_, status = Process.wait2(pid)
	assert_predicate status, :success?

[fatkodima]

Thanks! Applied all the suggestions.