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

Implement Socket#reuse_port on Windows #13326

Merged
merged 7 commits into from
Apr 18, 2023

Conversation

stakach
Copy link
Contributor

@stakach stakach commented Apr 16, 2023

I think this is a better solution than having runtime errors on windows and having to write code like this for multi-platform support

    # example of what needs to be written before this commit is merged
    @socket = UDPSocket.new @address.family
    @socket.reuse_address = true
    {% unless flag?(:win32) %}
      @socket.reuse_port = true
    {% end %}

noting that windows SO_REUSEADDR == linux SO_REUSEPORT
and on windows, reuse of the address component of a binding is always allowed

I've also changed the socket defaults to use SO_EXCLUSIVEADDRUSE
as per the docs:

image

image

@stakach stakach changed the title reuse_port on windows is the same as reuse_address reuse_address on windows is the same as reuse_port on linux Apr 17, 2023
@stakach stakach changed the title reuse_address on windows is the same as reuse_port on linux reuse_address on windows == reuse_port on linux Apr 17, 2023
@HertzDevil
Copy link
Contributor

HertzDevil commented Apr 17, 2023

Does this PR unblock the following spec?

pending_win32 "reuses the TCP port (SO_REUSEPORT)" do
s1 = HTTP::Server.new { |ctx| }
address = s1.bind_unused_port(reuse_port: true)
s2 = HTTP::Server.new { |ctx| }
s2.bind_tcp(address.port, reuse_port: true)
s1.close
s2.close
end

Also this error shows up on CI:

Unhandled exception in spawn: setsockopt 4: An invalid argument was supplied. (Socket::Error)
  from src\crystal\system\win32\socket.cr:322 in 'system_setsockopt'
  from src\socket.cr:403 in 'setsockopt'
  from src\socket.cr:413 in 'setsockopt_bool'
  from src\socket.cr:411 in 'setsockopt_bool'
  from src\socket.cr:275 in 'reuse_address='
  from src\socket\tcp_server.cr:40 in 'initialize:reuse_port'
  from src\socket\tcp_server.cr:36 in 'new:reuse_port'
  from src\http\server.cr:213 in 'bind_tcp'
  from spec\std\http\web_socket_spec.cr:377 in '->'
  from src\fiber.cr:146 in 'run'
  from src\fiber.cr:98 in '->'

@stakach
Copy link
Contributor Author

stakach commented Apr 17, 2023

@HertzDevil in regards to the error

  from src\socket.cr:411 in 'setsockopt_bool'
  from src\socket.cr:275 in 'reuse_address='

it looks like my macro finished isn't working as reuse_address should be a no-op

It should also unblock that http server spec reuses the TCP port I'll update that spec

@straight-shoota
Copy link
Member

Yeah macro finished didn't work as an override the way you had implemented it. It would define these methods after everything else, but still in the module scope. So it could not override the defs in the including scope.

@stakach
Copy link
Contributor Author

stakach commented Apr 17, 2023

needed macro included; macro finished switched to @HertzDevil's wasm approach
Specs passing now which is nice

@straight-shoota straight-shoota added this to the 1.9.0 milestone Apr 17, 2023
@straight-shoota straight-shoota changed the title reuse_address on windows == reuse_port on linux Implement Socket reuse_port (=reuse_address) on Windows Apr 18, 2023
@straight-shoota straight-shoota merged commit 949efb7 into crystal-lang:master Apr 18, 2023
@stakach stakach deleted the patch-9 branch April 18, 2023 22:43
@straight-shoota straight-shoota changed the title Implement Socket reuse_port (=reuse_address) on Windows Implement Socket#reuse_port on Windows Jul 10, 2023
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants