Skip to content

Resume zstream if available [Bug #10961] #22

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

Merged
merged 1 commit into from
Mar 8, 2021

Conversation

wanabe
Copy link
Contributor

@wanabe wanabe commented Mar 7, 2021

I think zstream should resume when a signal is received but no exception is raised.
I wanted to add tests, but it would consume too much time, so I decided to give up.

script bug10961.rb

require 'zlib'
Signal.trap("INT") { } if ARGV[0]
str = ("A".."z").to_a.join("") * 10_000
loop do
  str << str.dup
  spawn("ruby", "-e", <<~"end;", $$.to_s) # it must be out of GVL
    sleep 0.1
    begin
      Process.kill :INT, ARGV[0].to_i
    rescue Errno::ESRCH
    end
  end;
  t = Time.now
  Zlib::Deflate.deflate(str)
  s = Time.now - t
  puts "byte: %d, sec: %.4f" % [str.size, s]
  break if s > 0.5
end

trap without patch:

$ ruby -Ilib bug10961.rb trap
byte: 1160000, sec: 0.0070
byte: 2320000, sec: 0.0194
byte: 4640000, sec: 0.0393
byte: 9280000, sec: 0.0814
Traceback (most recent call last):
	3: from bug10961.rb:4:in `<main>'
	2: from bug10961.rb:4:in `loop'
	1: from bug10961.rb:14:in `block in <main>'
bug10961.rb:14:in `deflate': data error (Zlib::DataError)

with patch:

$ ruby -Ilib bug10961.rb trap
byte: 1160000, sec: 0.0061
byte: 2320000, sec: 0.0200
byte: 4640000, sec: 0.0389
byte: 9280000, sec: 0.0690
byte: 18560000, sec: 0.1063
byte: 37120000, sec: 0.2201
byte: 74240000, sec: 0.3873
byte: 148480000, sec: 0.7742

And the results are same when it is not trapped.
notrap without patch:

$ ruby -Ilib bug10961.rb
byte: 1160000, sec: 0.0060
byte: 2320000, sec: 0.0170
byte: 4640000, sec: 0.0378
byte: 9280000, sec: 0.0849
Traceback (most recent call last):
	3: from bug10961.rb:4:in `<main>'
	2: from bug10961.rb:4:in `loop'
	1: from bug10961.rb:14:in `block in <main>'
bug10961.rb:14:in `deflate': Interrupt
	3: from bug10961.rb:4:in `<main>'
	2: from bug10961.rb:4:in `loop'
	1: from bug10961.rb:14:in `block in <main>'
bug10961.rb:14:in `deflate': data error (Zlib::DataError)
bug10961.rb: Interrupt

notrap with patch:

$ ruby -Ilib bug10961.rb
byte: 1160000, sec: 0.0060
byte: 2320000, sec: 0.0198
byte: 4640000, sec: 0.0430
byte: 9280000, sec: 0.0786
Traceback (most recent call last):
	3: from bug10961.rb:4:in `<main>'
	2: from bug10961.rb:4:in `loop'
	1: from bug10961.rb:14:in `block in <main>'
bug10961.rb:14:in `deflate': Interrupt
	3: from bug10961.rb:4:in `<main>'
	2: from bug10961.rb:4:in `loop'
	1: from bug10961.rb:14:in `block in <main>'
bug10961.rb:14:in `deflate': data error (Zlib::DataError)

@wanabe
Copy link
Contributor Author

wanabe commented Mar 8, 2021

Thank you to review!
I'll merge it with "Rebase and merge" button.

@wanabe wanabe merged commit 785d747 into ruby:master Mar 8, 2021
@wanabe wanabe deleted the resume-stream branch March 8, 2021 22:26
@jorgemanrubia
Copy link

jorgemanrubia commented Mar 10, 2021

Hi @jeremyevans and @wanabe , any chance you could push a new rubygem version with this patch?

@jeremyevans
Copy link
Contributor

I haven't handled a zlib gem release, I think @hsbt handles them. I have no problems with a release, but it would be up to him. It appears the last gem release was before Ruby 2.7.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants