socketry / async-websocket

Asynchronous WebSocket client and server, supporting HTTP/1 and HTTP/2 for Ruby.
MIT License
166 stars 18 forks source link

transient Async::Pool::Controller stays when the connection is closed #46

Closed PeterRunich closed 1 year ago

PeterRunich commented 1 year ago
#!/usr/bin/env -S falcon serve --bind http://localhost:7070 --count 1 -c
require 'async/websocket/adapters/rack'

run lambda { |env|
  Async::WebSocket::Adapters::Rack.open(env, protocols: ['ws']) do |connection|
    connection.send_close 4000
  end
}

I use this server for testing

require 'async'
require 'async/websocket'
require 'async/http'

Async do |task|
  begin
    endpoint = Async::HTTP::Endpoint.parse 'ws://localhost:7070/'
    connection = Async::WebSocket::Client.connect endpoint
    connection.read
  rescue
    connection.close
  end

  p task.print_hierarchy
end

After printing tasks hierarchy i got this

#<Async::Task:0x0000000000000280 connected to #<Addrinfo: [::1]:7070 TCP (localhost)> [fd=7] (running)>
→ /home/pepe/.rbenv/versions/3.2.0/lib/ruby/gems/3.2.0/gems/async-2.3.1/lib/async/task.rb:68:in `backtrace'
  /home/pepe/.rbenv/versions/3.2.0/lib/ruby/gems/3.2.0/gems/async-2.3.1/lib/async/task.rb:68:in `backtrace'
  /home/pepe/.rbenv/versions/3.2.0/lib/ruby/gems/3.2.0/gems/async-2.3.1/lib/async/node.rb:268:in `print_backtrace'
  /home/pepe/.rbenv/versions/3.2.0/lib/ruby/gems/3.2.0/gems/async-2.3.1/lib/async/node.rb:261:in `block in print_hierarchy'
  /home/pepe/.rbenv/versions/3.2.0/lib/ruby/gems/3.2.0/gems/async-2.3.1/lib/async/node.rb:218:in `traverse_recurse'
  /home/pepe/.rbenv/versions/3.2.0/lib/ruby/gems/3.2.0/gems/async-2.3.1/lib/async/node.rb:214:in `traverse'
  /home/pepe/.rbenv/versions/3.2.0/lib/ruby/gems/3.2.0/gems/async-2.3.1/lib/async/node.rb:256:in `print_hierarchy'
  main.rb:71:in `block in <main>'
  /home/pepe/.rbenv/versions/3.2.0/lib/ruby/gems/3.2.0/gems/async-2.3.1/lib/async/task.rb:107:in `block in run'
  /home/pepe/.rbenv/versions/3.2.0/lib/ruby/gems/3.2.0/gems/async-2.3.1/lib/async/task.rb:248:in `block in schedule'
    #<Async::Task:0x0000000000000294 transient Async::Pool::Controller Gardener (running)>
    → /home/pepe/.rbenv/versions/3.2.0/lib/ruby/gems/3.2.0/gems/async-2.3.1/lib/async/scheduler.rb:79:in `transfer'
      /home/pepe/.rbenv/versions/3.2.0/lib/ruby/gems/3.2.0/gems/async-2.3.1/lib/async/scheduler.rb:79:in `transfer'
      /home/pepe/.rbenv/versions/3.2.0/lib/ruby/gems/3.2.0/gems/async-2.3.1/lib/async/task.rb:45:in `yield'
      /home/pepe/.rbenv/versions/3.2.0/lib/ruby/gems/3.2.0/gems/async-pool-0.3.12/lib/async/pool/controller.rb:184:in `block in start_gardener'
      /home/pepe/.rbenv/versions/3.2.0/lib/ruby/gems/3.2.0/gems/async-2.3.1/lib/async/task.rb:107:in `block in run'
      /home/pepe/.rbenv/versions/3.2.0/lib/ruby/gems/3.2.0/gems/async-2.3.1/lib/async/task.rb:248:in `block in schedule'
#<Async::Children:0x2a8 size=1>

i expect that Async::Pool::Controller Gardener task was gone after connection.close, but it there

i do some fix and seems it works https://github.com/socketry/async-websocket/pull/45

#<Async::Task:0x0000000000000a28 connected to #<Addrinfo: [::1]:7070 TCP (localhost)> [fd=7] (running)>
→ /home/pepe/.rbenv/versions/3.2.0/lib/ruby/gems/3.2.0/gems/async-2.3.0/lib/async/task.rb:68:in `backtrace'
  /home/pepe/.rbenv/versions/3.2.0/lib/ruby/gems/3.2.0/gems/async-2.3.0/lib/async/task.rb:68:in `backtrace'
  /home/pepe/.rbenv/versions/3.2.0/lib/ruby/gems/3.2.0/gems/async-2.3.0/lib/async/node.rb:269:in `print_backtrace'
  /home/pepe/.rbenv/versions/3.2.0/lib/ruby/gems/3.2.0/gems/async-2.3.0/lib/async/node.rb:262:in `block in print_hierarchy'
  /home/pepe/.rbenv/versions/3.2.0/lib/ruby/gems/3.2.0/gems/async-2.3.0/lib/async/node.rb:219:in `traverse_recurse'
  /home/pepe/.rbenv/versions/3.2.0/lib/ruby/gems/3.2.0/gems/async-2.3.0/lib/async/node.rb:215:in `traverse'
  /home/pepe/.rbenv/versions/3.2.0/lib/ruby/gems/3.2.0/gems/async-2.3.0/lib/async/node.rb:257:in `print_hierarchy'
  main.rb:71:in `block in <main>'
  /home/pepe/.rbenv/versions/3.2.0/lib/ruby/gems/3.2.0/gems/async-2.3.0/lib/async/task.rb:107:in `block in run'
  /home/pepe/.rbenv/versions/3.2.0/lib/ruby/gems/3.2.0/gems/async-2.3.0/lib/async/task.rb:248:in `block in schedule'
#<Async::Children size=0>

Is the behavior in the original considered okay? If "yes", can you explain why?

ioquatix commented 1 year ago

I will review your issue, however at first glance, isn't this wrong?

  rescue
    connection.close

shouldn't it be ensure?

PeterRunich commented 1 year ago

Сode in rescue block is executed. The idea is that on the server I close the connection with an error, I catch all errors, even though I could use Async::WebSocket::ProtocolError, but then it won't catch the error, because it throws Errno::EPIPE: Broken pipe.

require 'async'
require 'async/websocket'
require 'async/http'

Async do |task|
  begin
    endpoint = Async::HTTP::Endpoint.parse 'ws://localhost:7070/'
    connection = Async::WebSocket::Client.connect endpoint
    connection.read
  rescue Async::WebSocket::ProtocolError => e
    p e.code
    connection.close
  end

  p task.print_hierarchy
end
            | Task may have ended with unhandled exception.
               |   Errno::EPIPE: Broken pipe
               |   → /home/pepe/.rbenv/versions/3.2.0/lib/ruby/3.2.0/socket.rb:460 in `__write_nonblock'
               |     /home/pepe/.rbenv/versions/3.2.0/lib/ruby/3.2.0/socket.rb:460 in `write_nonblock'
               |     /home/pepe/.rbenv/versions/3.2.0/lib/ruby/gems/3.2.0/gems/async-io-1.34.0/lib/async/io/generic.rb:216 in `async_send'
               |     /home/pepe/.rbenv/versions/3.2.0/lib/ruby/gems/3.2.0/gems/async-io-1.34.0/lib/async/io/generic.rb:62 in `block in wrap_blocking_method'
               |     /home/pepe/.rbenv/versions/3.2.0/lib/ruby/gems/3.2.0/gems/async-io-1.34.0/lib/async/io/generic.rb:156 in `write'
               |     /home/pepe/.rbenv/versions/3.2.0/lib/ruby/gems/3.2.0/gems/async-io-1.34.0/lib/async/io/stream.rb:160 in `block in flush'
               |     /home/pepe/.rbenv/versions/3.2.0/lib/ruby/gems/3.2.0/gems/async-2.3.1/lib/async/semaphore.rb:68 in `acquire'
               |     /home/pepe/.rbenv/versions/3.2.0/lib/ruby/gems/3.2.0/gems/async-io-1.34.0/lib/async/io/stream.rb:155 in `flush'
               |     /home/pepe/.rbenv/versions/3.2.0/lib/ruby/gems/3.2.0/gems/protocol-websocket-0.9.1/lib/protocol/websocket/framer.rb:57 in `flush'
               |     /home/pepe/.rbenv/versions/3.2.0/lib/ruby/gems/3.2.0/gems/protocol-websocket-0.9.1/lib/protocol/websocket/connection.rb:68 in `flush'
               |     /home/pepe/.rbenv/versions/3.2.0/lib/ruby/gems/3.2.0/gems/protocol-websocket-0.9.1/lib/protocol/websocket/connection.rb:207 in `send_close'
               |     /home/pepe/.rbenv/versions/3.2.0/lib/ruby/gems/3.2.0/gems/protocol-websocket-0.9.1/lib/protocol/websocket/connection.rb:96 in `rescue in read_frame'
               |     /home/pepe/.rbenv/versions/3.2.0/lib/ruby/gems/3.2.0/gems/protocol-websocket-0.9.1/lib/protocol/websocket/connection.rb:81 in `read_frame'
               |     /home/pepe/.rbenv/versions/3.2.0/lib/ruby/gems/3.2.0/gems/protocol-websocket-0.9.1/lib/protocol/websocket/connection.rb:237 in `read'
               |     main.rb:142 in `block in <main>'
               |     /home/pepe/.rbenv/versions/3.2.0/lib/ruby/gems/3.2.0/gems/async-2.3.1/lib/async/task.rb:107 in `block in run'
               |     /home/pepe/.rbenv/versions/3.2.0/lib/ruby/gems/3.2.0/gems/async-2.3.1/lib/async/task.rb:248 in `block in schedule'
               |   Caused by Protocol::WebSocket::ClosedError:
               |   → /home/pepe/.rbenv/versions/3.2.0/lib/ruby/gems/3.2.0/gems/protocol-websocket-0.9.1/lib/protocol/websocket/connection.rb:143 in `receive_close'
               |     /home/pepe/.rbenv/versions/3.2.0/lib/ruby/gems/3.2.0/gems/protocol-websocket-0.9.1/lib/protocol/websocket/close_frame.rb:70 in `apply'
               |     /home/pepe/.rbenv/versions/3.2.0/lib/ruby/gems/3.2.0/gems/protocol-websocket-0.9.1/lib/protocol/websocket/connection.rb:92 in `read_frame'
               |     /home/pepe/.rbenv/versions/3.2.0/lib/ruby/gems/3.2.0/gems/protocol-websocket-0.9.1/lib/protocol/websocket/connection.rb:237 in `read'
               |     main.rb:142 in `block in <main>'
               |     /home/pepe/.rbenv/versions/3.2.0/lib/ruby/gems/3.2.0/gems/async-2.3.1/lib/async/task.rb:107 in `block in run'
               |     /home/pepe/.rbenv/versions/3.2.0/lib/ruby/gems/3.2.0/gems/async-2.3.1/lib/async/task.rb:248 in `block in schedule'
PeterRunich commented 1 year ago

@ioquatix if there’s anything I can do to help, let me know.

ioquatix commented 1 year ago

Can you please test out the latest version and confirm the issue is fixed? Thanks!

PeterRunich commented 1 year ago

Yep, it works as i expected, thanks for review and async gem!