Skip to content
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

Buffered Socket#close may not return if TCP send buffer is full #15129

Open
spuun opened this issue Oct 25, 2024 · 2 comments
Open

Buffered Socket#close may not return if TCP send buffer is full #15129

spuun opened this issue Oct 25, 2024 · 2 comments
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib:networking

Comments

@spuun
Copy link
Contributor

spuun commented Oct 25, 2024

Bug Report

In LavinMQ it's possible to force close client connections from the broker via the HTTP API. Today I stumbled over a bug where the HTTP request timed out, and after digging my conclusion is that it's because Socket#close never returned.

In this particular case it was a client that didn't process the messages LavinMQ sent to it, causing the TCP send buffer to fill up. The reason Socket#close doesn't return is because the write buffer in Socket contains data that #close wants to flush, which is impossible because the TCP buffer is full.

My first attempt was to use Buffered#unbuffered_close which as a first glance sounded like a solution, but Socket implement it as private. Instead i could use Socket#write_close.

PoC:

require "socket"
TCPServer.open(0) do |server|
  spawn { sleep if client = server.accept? }

  TCPSocket.open(server.local_address.address, server.local_address.port) do |client|
    client.sync = false

    ch = Channel(Nil).new
    spawn do
      loop do
        select
        when ch.receive
        when timeout 1.seconds
          # Ensure data in write buffer
          client.write "foo".to_slice if client.@out_count.zero?
          puts "Buffer full, closing socket"
          client.close
          # Never printed:
          puts "Socket closed"
        end
      end
    end

    data = Bytes.new(client.buffer_size // 2)
    loop do
      client.write data
      ch.send nil
    end
  end
end

Dunno if this should be treated as a bug, or a missing feature?

Maybe a #close(flush : Bool = true) is needed?

@spuun spuun added the kind:bug A bug in the code. Does not apply to documentation, specs, etc. label Oct 25, 2024
@ysbaddaden
Copy link
Contributor

Also maybe a timeout on the flush itself wouldn't hurt?

@spuun
Copy link
Contributor Author

spuun commented Oct 25, 2024

Also maybe a timeout on the flush itself wouldn't hurt?

It wouldn't!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. topic:stdlib:networking
Projects
None yet
Development

No branches or pull requests

3 participants