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

no overload matches 'Socket::Addrinfo.resolve' #10965

Closed
tleydxdy opened this issue Jul 19, 2021 · 12 comments · Fixed by #11049
Closed

no overload matches 'Socket::Addrinfo.resolve' #10965

tleydxdy opened this issue Jul 19, 2021 · 12 comments · Fixed by #11049

Comments

@tleydxdy
Copy link

When rebuilding my code with the updated crystal 1.1.0 it pops up with this error, which seems to be from the socket library itself. I'm using the debain repo from obs

In /usr/share/crystal/src/socket/addrinfo.cr:192:7

 192 | resolve(domain, service, family, Type::STREAM, Protocol::TCP) { |addrinfo| yield addrinfo }
       ^------
Error: no overload matches 'Socket::Addrinfo.resolve' with types Int32, Socket::Family, Bool, Socket::Type, Socket::Protocol

Overloads are:
 - Socket::Addrinfo.resolve(domain, service, family : Family | ::Nil = nil, type : Type = nil, protocol : Protocol = Protocol::IP, timeout = nil)
 - Socket::Addrinfo.resolve(domain, service, family : Family | ::Nil = nil, type : Type = nil, protocol : Protocol = Protocol::IP, timeout = nil, &block)
@asterite
Copy link
Member

What was the previous version you were using? It seems the code didn't compile either before, family is Bool in your code but it should be Family or Nil

@tleydxdy
Copy link
Author

it works fine with 1.0.0, I guess it was something in our code afterall. I tried to look into the changelogs/issues see if there was anything seemingly related :(
iv-org/invidious#2256

@straight-shoota
Copy link
Member

It looks like the custom TCPSocket#initialize somehow method overrides the initializer receiving a file descriptor.
I don't know why, though.

https://github.com/iv-org/invidious/blob/0d57a887ea59b6111d9a19b1ea1cea619ad9f129/src/invidious/helpers/helpers.cr#L763-L773

Adding proper type restrictions to the parameters should probably fix this.

@straight-shoota
Copy link
Member

straight-shoota commented Jul 19, 2021

From the error trace it seems this is what's happening:

Error Trace
In src/invidious.cr:37:22

 37 | PG_DB           = DB.open CONFIG.database_url
                           ^---
Error: instantiating 'DB:Module#open(URI)'


In lib/db/src/db.cr:119:5

 119 | build_database(uri)
       ^-------------
Error: instantiating 'build_database(URI)'


In lib/db/src/db.cr:155:14

 155 | Database.new(build_driver(uri), uri)
                ^--
Error: instantiating 'DB::Database.class#new(DB::Driver+, URI)'


In lib/db/src/db/database.cr:57:24

 57 | conn = @driver.build_connection(self).as(Connection)
                     ^---------------
Error: instantiating 'DB::Driver+#build_connection(DB::Database)'


In lib/pg/src/pg/driver.cr:3:16

 3 | Connection.new(context)
                ^--
Error: instantiating 'PG::Connection.class#new(DB::Database)'


In lib/pg/src/pg/connection.cr:13:38

 13 | @connection = PQ::Connection.new(conn_info)
                                   ^--
Error: instantiating 'PQ::Connection.class#new(PQ::ConnInfo)'


In lib/pg/src/pq/connection.cr:30:27

 30 | soc = TCPSocket.new(@conninfo.host, @conninfo.port)
                      ^--
Error: instantiating 'TCPSocket.class#new(String, Int32)'


In src/invidious/helpers/helpers.cr:764:3

 764 | def initialize(host, port, dns_timeout = nil, connect_timeout = nil, family = Socket::Family::UNSPEC)
       ^---------
Error: instantiating 'initialize(String, Int32, Nil, Nil, Socket::Family)'


In src/invidious/helpers/helpers.cr:765:14

 765 | Addrinfo.tcp(host, port, timeout: dns_timeout, family: family) do |addrinfo|
                ^--
Error: instantiating 'Socket::Addrinfo.class#tcp(String, Int32)'


In /usr/share/crystal/src/socket/addrinfo.cr:192:7

 192 | resolve(domain, service, family, Type::STREAM, Protocol::TCP) { |addrinfo| yield addrinfo }
       ^------
Error: instantiating 'resolve(String, Int32, Socket::Family, Socket::Type, Socket::Protocol)'


In /usr/share/crystal/src/socket/addrinfo.cr:60:7

 60 | getaddrinfo(domain, service, family, type, protocol, timeout) do |addrinfo|
      ^----------
Error: instantiating 'getaddrinfo(String, Int32, Socket::Family, Socket::Type, Socket::Protocol, Nil)'


In /usr/share/crystal/src/socket/addrinfo.cr:60:7

 60 | getaddrinfo(domain, service, family, type, protocol, timeout) do |addrinfo|
      ^----------
Error: instantiating 'getaddrinfo(String, Int32, Socket::Family, Socket::Type, Socket::Protocol, Nil)'


In /usr/share/crystal/src/socket/addrinfo.cr:63:9

 63 | loop do
      ^---
Error: instantiating 'loop()'


In /usr/share/crystal/src/socket/addrinfo.cr:63:9

 63 | loop do
      ^---
Error: instantiating 'loop()'


In /usr/share/crystal/src/socket/addrinfo.cr:192:7

 192 | resolve(domain, service, family, Type::STREAM, Protocol::TCP) { |addrinfo| yield addrinfo }
       ^------
Error: instantiating 'resolve(String, Int32, Socket::Family, Socket::Type, Socket::Protocol)'


In src/invidious/helpers/helpers.cr:765:14

 765 | Addrinfo.tcp(host, port, timeout: dns_timeout, family: family) do |addrinfo|
                ^--
Error: instantiating 'Socket::Addrinfo.class#tcp(String, Int32)'


In src/invidious/helpers/helpers.cr:766:7

 766 | super(addrinfo.family, addrinfo.type, addrinfo.protocol)
       ^----
Error: instantiating 'super(Socket::Family, Socket::Type, Socket::Protocol)'


In /usr/share/crystal/src/socket.cr:49:5

 49 | initialize(fd, family, type, protocol, blocking)
      ^---------
Error: instantiating 'initialize(Int32, Socket::Family, Socket::Type, Socket::Protocol, Bool)'


In src/invidious/helpers/helpers.cr:765:14

 765 | Addrinfo.tcp(host, port, timeout: dns_timeout, family: family) do |addrinfo|
                ^--
Error: instantiating 'Socket::Addrinfo.class#tcp(Int32, Socket::Family)'


In /usr/share/crystal/src/socket/addrinfo.cr:192:7

 192 | resolve(domain, service, family, Type::STREAM, Protocol::TCP) { |addrinfo| yield addrinfo }
       ^------
Error: no overload matches 'Socket::Addrinfo.resolve' with types Int32, Socket::Family, Bool, Socket::Type, Socket::Protocol

Overloads are:
 - Socket::Addrinfo.resolve(domain, service, family : Family | ::Nil = nil, type : Type = nil, protocol : Protocol = Protocol::IP, timeout = nil)
 - Socket::Addrinfo.resolve(domain, service, family : Family | ::Nil = nil, type : Type = nil, protocol : Protocol = Protocol::IP, timeout = nil, &block)

instantiating 'initialize(Int32, Socket::Family, Socket::Type, Socket::Protocol, Bool)' is supposed to hit this initializer:

def initialize(fd, @family : Family, @type : Type, @protocol : Protocol = Protocol::IP, blocking = false) (socket.cr)

But the custom initializer added as helper in invidious seems to override that for some reason:

def initialize(host, port, dns_timeout = nil, connect_timeout = nil, family = Socket::Family::UNSPEC) (helpers.cr)

EDIT: The reason is that Socket initializers are hidden in TCPSocket and TCPSocket's implementation of that initializer does not have a blocking parameter, thus the call signature does not match.

@tleydxdy
Copy link
Author

I have fixed this issue, then another issue crops up, text.rindex want to return a Int32 but the number given to it was explicitly Int64. you can see the patch https://github.com/iv-org/invidious/pull/2263/files#diff-c7fb42237f52a2684edf3d22d5f0b36f7f0ac8940f71c53886fe4a294ef73357L135

I wonder why isn't these issues showing up before. did something changed in the type inferencer? or it's simply a bug got fixed.

@straight-shoota
Copy link
Member

@tleydxdy I have created a new issue for String#rindex. That seems like a backwards-incompatible change slipped through.

There were some changes to the way method overloads are matched, they were supposed to be only improvements fixing incorrect behaviour. I'm not yet sure about the reason for your issue with the TCPSocket initializer. It doesn't look right what the compiler is doing there.

@SamantazFox
Copy link

SamantazFox commented Aug 1, 2021

There seem to be a huge inheritance problem here.

From the log provided by @straight-shoota:

  1. We hit our overridle of TCPSocket.initialize() (in helpers.cr, L765) => OK
  2. We hit Addrinfo.tcp(...) (the following line) => OK
  3. We hit Addrinfo.resolve(...) (from src/socket/addrinfo.cr, L192) => OK
  4. It yield addrinfo, so for each result we hit the following:
    a. getaddrinfo() do |addrinfo| => OK
    b. loop do => OK
  5. We return from Addrinfo.resolve(...) (at src/socket/addrinfo.cr, L192)
  6. We return from Addrinfo.tcp(...) (at helpers.cr, L765)

So up to that part, we're good. But then:

  1. We call super(), so we should hit Socket.initialize()
  2. Which we do (src/socket.cr, L44-50)
  3. And then, we hit the other initialize function (the one which takes a file descriptor, src/socket.cr, L49)

And finally, we hit Addrinfo.tcp() (from helpers.cr L765) again?

It looks like calling initialize from Socket.initialize used TCPSocket.initialize, hence passing a file descriptor (Int32) to TCPSocket.new, which in turn sends it to Addrinfo.resolve, hence the error.

Correct me if my logic is wrong.


Edit: sorry if that's hard to read, I tried my best at formating

@SamantazFox
Copy link

And if it happens in this case, it may probably happen in other classes. This can possibly lead to endless loops when trying to compile.

@asterite
Copy link
Member

asterite commented Aug 1, 2021

I didn't read anything, but it's most likely a duplicate of #2293

SamantazFox added a commit to SamantazFox/invidious that referenced this issue Aug 2, 2021
@straight-shoota
Copy link
Member

straight-shoota commented Aug 2, 2021

@asterite Yes, that seems to be a contributing factor.

But it should actually not be relevant here, because the methods have different signatures. The custom TCPSocket#initialize from invidious' helpers.cr has no type restrictions whatsoever.
There's a Socket#initialize overload with matching type restrictions, but that one is not visible due to TCPSocket's initializers overshadowing all of Sockets (#4546, #4762). There is a similar initializer on TCPSocket that delegates to the super implementation, but it's missing the blocking parameter:

protected def initialize(fd : Handle, family : Family, type : Type, protocol : Protocol)
super fd, family, type, protocol
end

And indeed, applying this patch to stdlib fixes the invidious build error:

--- i/src/socket/tcp_socket.cr
+++ w/src/socket/tcp_socket.cr
@@ -38,8 +38,8 @@ class TCPSocket < IPSocket
     super family, type, protocol
   end

-  protected def initialize(fd : Handle, family : Family, type : Type, protocol : Protocol)
-    super fd, family, type, protocol
+  protected def initialize(fd : Handle, family : Family, type : Type, protocol : Protocol, blocking = false)
+    super fd, family, type, protocol, blocking
   end

   # Creates a TCPSocket from an already configured raw file descriptor

The reason for this particular failure was that the blocking parameter was added to Socket#initialize at a later time and was not copied over to the delegating TCPSocket#initialize.

@straight-shoota
Copy link
Member

An overrides annotation could help avoiding such issues when those inherited constructors don't override the complete super method signature (see #1647). It's a very special case, though.

@SamantazFox
Copy link

I didn't read anything, but it's most likely a duplicate of #2293

Yep, that looks very similar.

Ideally, calling initialize in some class shouldn't be able to loop back to sub-classes. But not for other methods.

SamantazFox added a commit to iv-org/invidious that referenced this issue Aug 3, 2021
* Move Crystal stdlib classes overrides to a separate file
* Document known crystal overrides
* Update crystal overrides for HTTP::Client socket
* Update shard.yml to restrict crystal versions
* Fix compilation error in Crystal 1.1.x (See
   crystal-lang/crystal#10965
   for more details about this issue).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants