Skip to content

Commit

Permalink
Avoid mutating stdout and stderr
Browse files Browse the repository at this point in the history
Uses `flush` after every write if the stdio it is not synchronized.

Closes: puma#1948
  • Loading branch information
Carlos Castellanos Vera committed Nov 18, 2020
1 parent a7c931a commit 52a563a
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 8 deletions.
1 change: 1 addition & 0 deletions History.md
Expand Up @@ -2,6 +2,7 @@

* Features
* Your feature goes here <Most recent on the top, like GitHub> (#Github Number)
* Uses stdio `flush` to avoiding mutating $stdout and $stderr `with sync=true` ([#2486])
* Prints the loaded configuration if the environment variable `PUMA_LOG_CONFIG` is present ([#2472])
* Integrate with systemd's watchdog and notification features ([#2438])
* Adds max_fast_inline as a configuration option for the Server object ([#2406])
Expand Down
13 changes: 10 additions & 3 deletions lib/puma/error_logger.rb
Expand Up @@ -15,7 +15,6 @@ class ErrorLogger

def initialize(ioerr)
@ioerr = ioerr
@ioerr.sync = true

@debug = ENV.key? 'PUMA_DEBUG'
end
Expand All @@ -32,7 +31,7 @@ def self.stdio
# and before all remaining info.
#
def info(options={})
ioerr.puts title(options)
log title(options)
end

# Print occured error details only if
Expand All @@ -54,7 +53,7 @@ def debug(options={})
string_block << request_dump(req) if request_parsed?(req)
string_block << error.backtrace if error

ioerr.puts string_block.join("\n")
log string_block.join("\n")
end

def title(options={})
Expand Down Expand Up @@ -93,5 +92,13 @@ def request_headers(req)
def request_parsed?(req)
req && req.env[REQUEST_METHOD]
end

private

def log(str)
ioerr.puts str

ioerr.flush unless ioerr.sync
end
end
end
5 changes: 2 additions & 3 deletions lib/puma/events.rb
Expand Up @@ -30,9 +30,6 @@ def initialize(stdout, stderr)
@stdout = stdout
@stderr = stderr

@stdout.sync = true
@stderr.sync = true

@debug = ENV.key? 'PUMA_DEBUG'
@error_logger = ErrorLogger.new(@stderr)

Expand Down Expand Up @@ -66,6 +63,8 @@ def register(hook, obj=nil, &blk)
#
def log(str)
@stdout.puts format(str) if @stdout.respond_to? :puts

@stdout.flush unless @stdout.sync
rescue Errno::EPIPE
end

Expand Down
4 changes: 2 additions & 2 deletions lib/puma/runner.rb
Expand Up @@ -106,8 +106,8 @@ def redirect_io
end

STDOUT.reopen stdout, (append ? "a" : "w")
STDOUT.sync = true
STDOUT.puts "=== puma startup: #{Time.now} ==="
STDOUT.flush unless STDOUT.sync
end

if stderr
Expand All @@ -116,8 +116,8 @@ def redirect_io
end

STDERR.reopen stderr, (append ? "a" : "w")
STDERR.sync = true
STDERR.puts "=== puma startup: #{Time.now} ==="
STDERR.flush unless STDERR.sync
end
end

Expand Down
8 changes: 8 additions & 0 deletions test/test_error_logger.rb
Expand Up @@ -10,6 +10,14 @@ def test_stdio
assert_equal STDERR, error_logger.ioerr
end


def test_stdio_respects_sync
error_logger = Puma::ErrorLogger.stdio

assert_equal STDERR.sync, error_logger.ioerr.sync
assert_equal STDERR, error_logger.ioerr
end

def test_info_with_only_error
_, err = capture_io do
Puma::ErrorLogger.stdio.info(error: StandardError.new('ready'))
Expand Down
9 changes: 9 additions & 0 deletions test/test_events.rb
Expand Up @@ -24,6 +24,15 @@ def test_stdio
assert_equal STDERR, events.stderr
end

def test_stdio_respects_sync
events = Puma::Events.stdio

assert_equal STDOUT.sync, events.stdout.sync
assert_equal STDERR.sync, events.stderr.sync
assert_equal STDOUT, events.stdout
assert_equal STDERR, events.stderr
end

def test_register_callback_with_block
res = false

Expand Down

0 comments on commit 52a563a

Please sign in to comment.