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
Fix "log_writer.rb:59:in `write': deadlock; recursive locking (ThreadError)" #2882
Conversation
@@ -56,14 +57,17 @@ def self.null | |||
|
|||
# Write +str+ to +@stdout+ | |||
def log(str) | |||
@stdout.is_a?(IO) and @stdout.wait_writable(1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://ruby-doc.org/stdlib-2.7.1/libdoc/io/wait/rdoc/IO.html#wait_writable-method
Waits until IO is writable without blocking and returns
self
ornil
when times out.
Sounds like this shouldn't hurt to have
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds like this shouldn't hurt to have
My thought also. Also, catching 'bad io' errors so a restart or shutdown isn't interrupted.
I intend to run it a few times in my fork to see if the intermittent error occurs...
…rror) This is happening in Signal.trap blocks, which cannot use Mutex, Monitor, etc for locking
Since this was merged, I've seen one "log_writer.rb:59:in `write': deadlock; recursive locking (ThreadError)" error running in my fork. The following doesn't afford really perfect locking, but it's better than what we have now. Any thoughts? It runs fine locally, haven't tried it on Actions... WRITE_QUEUE = Thread::Queue.new
def lock
locked = false
@stdout.is_a?(IO) and @stdout.wait_writable(1)
sleep 0.005 until WRITE_QUEUE.empty?
WRITE_QUEUE.push Process.pid
locked = true
yield
@stdout.flush unless @stdout.sync
rescue Errno::EPIPE, Errno::EBADF
ensure
WRITE_QUEUE.pop if locked
end
# Write +str+ to +@stdout+
def log(str)
lock { @stdout.puts(format str) } if @stdout.respond_to? :puts
end
def write(str)
lock { @stdout.write format(str) }
end |
…Error)" (puma#2882) * log_writer - try to fix `write': deadlock; recursive locking (ThreadError) This is happening in Signal.trap blocks, which cannot use Mutex, Monitor, etc for locking * Tests - minor changes for intermittent issues
Description
CI intermittently fails with "log_writer.rb:59:in `write': deadlock; recursive locking (ThreadError)". This is happening in log writes/puts, and this code maybe called in signal handlers. Mutex and other locking mechanisms cannot be used in signal handlers, so this PR is an attempt to fix the problem.
Example log:
While doing recent work in my fork I have also seen the error. Can't repo locally, and tried to get code independent of Puma to fail, again, no luck.
Wondering if anyone has a better idea?
Second commit contains minor test fixes for intermittent issues.
Your checklist for this pull request
[ci skip]
to the title of the PR.#issue
" to the PR description or my commit messages.