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

Error instantiating 'Socket#accept()' #6212

Closed
ghost opened this issue Jun 18, 2018 · 6 comments · Fixed by #6277
Closed

Error instantiating 'Socket#accept()' #6212

ghost opened this issue Jun 18, 2018 · 6 comments · Fixed by #6277
Labels
good first issue This is an issue suited for newcomers to become aquianted with working on the codebase. kind:bug

Comments

@ghost
Copy link

ghost commented Jun 18, 2018

require "socket"

server = Socket.new(Socket::Family::INET, Socket::Type::STREAM)
server.reuse_address = true

server.bind(1234)
server.listen

while true
  client = server.accept
end

This code gives -

Error in s.cr:10: instantiating 'Socket#accept()'

  client = server.accept
                  ^~~~~~

in /usr/share/crystal/src/socket.cr:212: instantiating 'accept?()'

    accept? || raise IO::Error.new("Closed stream")
    ^~~~~~~

in /usr/share/crystal/src/socket.cr:231: wrong number of arguments for 'Socket.new' (given 1, expected 2..5)Overloads are:
 - Socket.new(fd : Int32, family, type, protocol = Protocol::IP, blocking = false)
 - Socket.new(family, type, protocol = Protocol::IP, blocking = false)

      sock = Socket.new(client_fd)
                    ^~~
@straight-shoota
Copy link
Member

straight-shoota commented Jun 18, 2018

Socket#accept does not work. You need to use a Socket::Server which implements a custom #accept? method. In this case you could use TCPServer.

The Socket API is a real mess, unfortunately. But there is already an issue for refactoring that (#5778).

@ghost
Copy link
Author

ghost commented Jun 18, 2018

Okay, thank you, I will close this issue.

@ghost ghost closed this as completed Jun 18, 2018
@ysbaddaden
Copy link
Contributor

Reopening, this should work. We merely miss a Socket.new(fd) overload.

@ysbaddaden ysbaddaden reopened this Jun 18, 2018
@ysbaddaden ysbaddaden added kind:bug good first issue This is an issue suited for newcomers to become aquianted with working on the codebase. labels Jun 18, 2018
@Daniel-Worrall
Copy link
Contributor

Can't we instance the new socket with the family and type of the server using the existing overloads?

@RX14
Copy link
Contributor

RX14 commented Jun 25, 2018

getsockopt can query the SO_PROTOCOL and SO_TYPE, so a .new(fd) could be implemented.

However, it's not clear that we don't just want to copy the values from the socket you called accept from. I don't know the minutia of the socket API.

@straight-shoota
Copy link
Member

We can add this now, or just skip to #5778 and rework the entire sockets API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue This is an issue suited for newcomers to become aquianted with working on the codebase. kind:bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants