Track and wait on dead workers to prevent zombies #1884
Conversation
|
Should the test from #1864 be added too? |
|
I looked at this in both 3.12.1 and master with the test from #1864 added. Some interesting things, along with the issues that have appeared with Ruby 2.6+. Anyway, if the code in pids = []
while pid = Process.waitpid(-1, Process::WNOHANG) do
pids << pid
end
@workers.reject! { |w| w.dead? || pids.include?(w.pid) }Everything passes (with a few other minor changes), including the mentioned test, but Ruby 2.2 fails intermittently on the new zombie test. It's essentially a simplification of the code that existed in 3.12.1. but it does use '-1', which you've mentioned you'd prefer not to use... Top two commits at https://github.com/MSP-Greg/puma/commits/zombies. I deleted one that may make 2.2 failures less common. Travis test is: https://travis-ci.org/MSP-Greg/puma/builds/568144883 |
|
@MSP-Greg Oh great! Please open a PR against that branch and we'll use it. |
|
When you have a minute, a look at #1836 would be appreciated. The only real feature added is Otherwise, it's a few things that make SSL testing better, as there are some false positives, etc happening. Obviously, false positives are not good. Also, I've got a patch for the 'http & OpenSSL 1.1.1 (TLSv1.3)' problem, but I really would rather do it on top of #1836... Thanks, Greg EDIT: and, I tested 1836 locally, as everything in it affects all platforms... |
|
just pasting this comment here for my own review later: #1864 (comment) |
|
Closing in favor of #1887 |
This fixes a bug introduced in #1748 and supersedes the work in PR #1864.
The bug was the the old logic used
-1toProcess.waitpidwhich would catch any pids, including ones that we accidentally already removed from@workersbefore waiting on them.This logic tracks the dead workers separately so we're sure to wait on them.