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

NotImplementedError of Socket in Win32 #13328

Closed
yanecc opened this issue Apr 17, 2023 · 8 comments · Fixed by #13477
Closed

NotImplementedError of Socket in Win32 #13328

yanecc opened this issue Apr 17, 2023 · 8 comments · Fixed by #13477
Labels
kind:bug A bug in the code. Does not apply to documentation, specs, etc. platform:windows Windows support based on the MSVC toolchain / Win32 API topic:stdlib:networking

Comments

@yanecc
Copy link

yanecc commented Apr 17, 2023

It seems that Socket for win32 has been implemented. #10784
Compiling projects built with grip or kemal has no more problems other than the wrong newline character of the exception_page.ecr file. #13304
However, when I run it, NotImplementedError of Socket#reuse_port occurs.

Unhandled exception: Not Implemented: Socket#reuse_port= (NotImplementedError)
  from A:\Scoop\apps\crystal\current\src\crystal\system\win32\socket.cr:261 in 'system_reuse_port='
  from A:\Scoop\apps\crystal\current\src\socket.cr:275 in 'reuse_port='
  from A:\Scoop\apps\crystal\current\src\socket\tcp_server.cr:41 in 'initialize:reuse_port'
  from A:\Scoop\apps\crystal\current\src\socket\tcp_server.cr:36 in 'new:reuse_port'
  from A:\Scoop\apps\crystal\current\src\http\server.cr:213 in 'bind_tcp'
  from lib\grip\src\grip\application.cr:107 in 'run'
  from src\route.cr:43 in '__crystal_main'
  from A:\Scoop\apps\crystal\current\src\crystal\main.cr:115 in 'main_user_code'
  from A:\Scoop\apps\crystal\current\src\crystal\main.cr:101 in 'main'
  from A:\Scoop\apps\crystal\current\src\crystal\main.cr:127 in 'main'
  from A:\Scoop\apps\crystal\current\src\crystal\system\win32\wmain.cr:37 in 'wmain'
  from D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288 in '__scrt_common_main_seh'
  from C:\WINDOWS\System32\KERNEL32.DLL +75421 in 'BaseThreadInitThunk'
  from C:\WINDOWS\SYSTEM32\ntdll.dll +371192 in 'RtlUserThreadStart'

Crystal - 1.8.0

@yanecc yanecc added the kind:bug A bug in the code. Does not apply to documentation, specs, etc. label Apr 17, 2023
@Sija
Copy link
Contributor

Sija commented Apr 17, 2023

Related: #13326

@yanecc yanecc closed this as completed Apr 17, 2023
@yanecc
Copy link
Author

yanecc commented May 16, 2023

I reopen this issue duo to failed attempts with the probably new version to come.
Branch #13326 has been deleted, while
Runtime error exists as follow.

ERROR - http.server: Error while connecting a new socket
setsockopt: invalid argument (Socket::Error)
  from A:\Scoop\apps\crystal\current\src\socket.cr:60 in 'initialize'
  from A:\Scoop\apps\crystal\current\src\socket\tcp_server.cr:112 in 'accept?'
  from A:\Scoop\apps\crystal\current\src\http\server.cr:467 in '->'
  from A:\Scoop\apps\crystal\current\src\fiber.cr:146 in 'run'

@yanecc yanecc reopened this May 16, 2023
@yanecc
Copy link
Author

yanecc commented May 16, 2023

Similar to #9065, but only occurs on Windows in my use case.

@HertzDevil HertzDevil added topic:stdlib:networking platform:windows Windows support based on the MSVC toolchain / Win32 API labels May 16, 2023
@HertzDevil
Copy link
Contributor

Do you have a minimal reproducible example?

@yanecc
Copy link
Author

yanecc commented May 17, 2023

Do you have a minimal reproducible example?

require "http/server"

server = HTTP::Server.new do |context|
  case context.request.method
  when "GET"
    case context.request.path
    when "/time"
      time(context)
    else
      context.response.status = HTTP::Status::NOT_FOUND
      context.response.puts "Cannot #{context.request.method} #{context.request.path}"
    end
  else
    context.response.status = HTTP::Status::METHOD_NOT_ALLOWED
    context.response.puts "#{context.request.method} method is not supported"
  end
end

def time(context)
  time = Time.utc(1, 1, 1)
  context.response.puts time.to_s("%F")
end

address = server.bind_tcp "0.0.0.0", 80, true
puts "Listening on http://" + address.to_s
server.listen

On Windows

ERROR - http.server: Error while connecting a new socket
setsockopt: 提供了一个无效的参数。 (Socket::Error)
  from A:\Scoop\apps\crystal\current\src\crystal\system\win32\socket.cr:91 in 'initialize_handle'
  from A:\Scoop\apps\crystal\current\src\socket.cr:56 in 'initialize'
  from A:\Scoop\apps\crystal\current\src\socket\tcp_socket.cr:44 in 'initialize:fd:family:type:protocol'
  from A:\Scoop\apps\crystal\current\src\socket\tcp_socket.cr:43 in 'new:fd:family:type:protocol'
  from A:\Scoop\apps\crystal\current\src\socket\tcp_server.cr:112 in 'accept?'
  from A:\Scoop\apps\crystal\current\src\http\server.cr:461 in '->'
  from A:\Scoop\apps\crystal\current\src\fiber.cr:146 in 'run'
  from A:\Scoop\apps\crystal\current\src\fiber.cr:98 in '->'

On Alpine with Crystal 1.8.2, it returns as expected

0001-01-01

@yanecc
Copy link
Author

yanecc commented May 17, 2023

Also, with reuse_port set false, it works quite well.
Another fact is that shards run falls with

Unhandled exception: Error executing process: '/serve/bin/domain':  (IO::Error)
  from A:\Scoop\apps\crystal\current\shards.exe +144166 in '??'
  from A:\Scoop\apps\crystal\current\shards.exe +144062 in '??'
  from A:\Scoop\apps\crystal\current\shards.exe +144026 in '??'
  from A:\Scoop\apps\crystal\current\shards.exe +101263 in '??'
  from A:\Scoop\apps\crystal\current\shards.exe +1274520 in '??'
  from A:\Scoop\apps\crystal\current\shards.exe +899684 in '??'
  from A:\Scoop\apps\crystal\current\shards.exe +899411 in '??'
  from A:\Scoop\apps\crystal\current\shards.exe +915011 in '??'
  from A:\Scoop\apps\crystal\current\shards.exe +2317922 in '??'
  from A:\Scoop\apps\crystal\current\shards.exe +2316670 in '??'
  from A:\Scoop\apps\crystal\current\shards.exe +125030 in '??'
  from A:\Scoop\apps\crystal\current\shards.exe +1716482 in '??'
  from A:\Scoop\apps\crystal\current\shards.exe +1704261 in '??'
  from A:\Scoop\apps\crystal\current\shards.exe +5832 in '??'
  from A:\Scoop\apps\crystal\current\shards.exe +2346921 in '??'
  from A:\Scoop\apps\crystal\current\shards.exe +2346698 in '??'
  from A:\Scoop\apps\crystal\current\shards.exe +79097 in '??'
  from A:\Scoop\apps\crystal\current\shards.exe +79379 in '??'
  from A:\Scoop\apps\crystal\current\shards.exe +2844596 in '??'
  from C:\WINDOWS\System32\KERNEL32.DLL +75437 in 'BaseThreadInitThunk'
  from C:\WINDOWS\SYSTEM32\ntdll.dll +371192 in 'RtlUserThreadStart'

It's shards build that results the runtime error above.

@HertzDevil
Copy link
Contributor

From what I could gather, if SO_REUSEADDR has already been set then there is no need to set SO_EXCLUSIVEADDRUSE on Windows, but Crystal::System::Socket#initialize_handle is unconditionally setting the latter. Is that right? cc @stakach

@stakach
Copy link
Contributor

stakach commented May 17, 2023

From what I could gather, if SO_REUSEADDR has already been set then there is no need to set SO_EXCLUSIVEADDRUSE on Windows, but Crystal::System::Socket#initialize_handle is unconditionally setting the latter. Is that right? cc @stakach

Yeah my bad, they are exclusive settings - I didn't pay enough attention to the initialization flow. So your pull request looks good to me

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. platform:windows Windows support based on the MSVC toolchain / Win32 API topic:stdlib:networking
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants