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

Support for Socket.ip_address_list, Addrinfo, and a near total rewrite #9

Merged
merged 419 commits into from Dec 28, 2015

Conversation

Projects
None yet
6 participants
@YorickPeterse
Member

YorickPeterse commented Feb 24, 2015

This is still a work in progress, but once done should add support for Socket.ip_address_list.

Socket:

  • Socket.ip_address_list
  • Better tests (if possible) for Socket.ip_address_list

Addrinfo

  • Addrinfo.foreach
  • Addrinfo.getaddrinfo
  • Addrinfo.ip
  • Addrinfo.tcp
  • Addrinfo.udp
  • Addrinfo.unix
  • Addrinfo#initialize
  • Addrinfo#afamily
  • Addrinfo#bind
  • Addrinfo#canonname
  • Addrinfo#connect
  • Addrinfo#connect_from
  • Addrinfo#connect_to
  • Addrinfo#family_addrinfo
  • Addrinfo#getnameinfo
  • Addrinfo#inspect
  • Addrinfo#inspect_sockaddr
  • Addrinfo#ip?
  • Addrinfo#ip_address
  • Addrinfo#ip_port
  • Addrinfo#ip_unpack
  • Addrinfo#ipv4?
  • Addrinfo#ipv4_loopback?
  • Addrinfo#ipv4_multicast?
  • Addrinfo#ipv4_private?
  • Addrinfo#ipv6?
  • Addrinfo#ipv6_linklocal?
  • Addrinfo#ipv6_loopback?
  • Addrinfo#ipv6_mc_global?
  • Addrinfo#ipv6_mc_linklocal?
  • Addrinfo#ipv6_mc_nodelocal?
  • Addrinfo#ipv6_mc_orglocal?
  • Addrinfo#ipv6_mc_sitelocal?
  • Addrinfo#ipv6_multicast?
  • Addrinfo#ipv6_sitelocal?
  • Addrinfo#ipv6_to_ipv4
  • Addrinfo#ipv6_unique_local?
  • Addrinfo#ipv6_unspecified?
  • Addrinfo#ipv6_v4compat?
  • Addrinfo#ipv6_v4mapped?
  • Addrinfo#listen
  • Addrinfo#marshal_dump
  • Addrinfo#marshal_load
  • Addrinfo#pfamily
  • Addrinfo#protocol
  • Addrinfo#socktype
  • Addrinfo#to_s
  • Addrinfo#to_sockaddr
  • Addrinfo#unix?
  • Addrinfo#unix_path

Socket Spec Status

Specs and implementations of the following methods should be verified and optionally updated/re-factored should they not be compliant with MRI 2.x:

  • BasicSocket#close_read
  • BasicSocket#close_write
  • BasicSocket#connect_address
  • BasicSocket#do_not_reverse_lookup=
  • BasicSocket#do_not_reverse_lookup
  • BasicSocket#getpeereid
  • BasicSocket#getpeername
  • BasicSocket#getsockname
  • BasicSocket#getsockopt
  • BasicSocket#local_address
  • BasicSocket#recv_nonblock
  • BasicSocket#recv
  • BasicSocket#recvmsg_nonblock
  • BasicSocket#recvmsg
  • BasicSocket#remote_address
  • BasicSocket#send
  • BasicSocket#sendmsg_nonblock
  • BasicSocket#sendmsg
  • BasicSocket#setsockopt
  • BasicSocket#shutdown
  • BasicSocket.for_fd
  • IPSocket.getaddress
  • IPSocket#addr
  • IPSocket#peeraddr
  • IPSocket#recvfrom
  • Socket.getaddrinfo
  • Socket#accept_nonblock
  • Socket#accept
  • Socket#bind
  • Socket#connect_nonblock
  • Socket#connect
  • Socket#ipv6only!
  • Socket#listen
  • Socket#recvfrom_nonblock
  • Socket#recvfrom
  • Socket#sysaccept
  • TCPServer#accept_nonblock
  • TCPServer#accept
  • TCPServer#listen
  • TCPServer#sysaccept
  • UDPSocket#bind
  • UDPSocket#connect
  • UDPSocket#recvfrom_nonblock
  • UDPSocket#send
  • UNIXServer#accept_nonblock
  • UNIXServer#accept
  • UNIXServer#listen
  • UNIXServer#sysaccept
  • UNIXSocket#addr
  • UNIXSocket#path
  • UNIXSocket#peeraddr
  • UNIXSocket#recv_io
  • UNIXSocket#recvfrom
  • UNIXSocket#send_io
  • TCPSocket.gethostbyname
  • TCPSocket#initialize
  • TCPSocket#send
  • TCPSocket#local_address
  • TCPSocket#remote_address
  • Refactor TCPSocket#tcp_setup, removing it entirely if possible
  • IPSocket#recvfrom_nonblock

Output generated using:

ruby -r socket -e '[BasicSocket, Socket, IPSocket, TCPSocket, UDPSocket, UNIXSocket, TCPServer, UNIXServer].each { |const| puts const.instance_methods(false).map { |m| "#{const}##{m}" } }'

Socket::AncillaryData

  • Socket::AncillaryData.int
  • Socket::AncillaryData.unix_rights
  • Socket::AncillaryData.ip_pktinfo
  • Socket::AncillaryData.ipv6_pktinfo
  • Socket::AncillaryData#initialize
  • Socket::AncillaryData#inspect: not going to mimic this as the MRI implementation is a PITA (https://github.com/ruby/ruby/blob/d2e48a3f01d9ce2fa6fd860ad5bd2fee28e29e66/ext/socket/ancdata.c#L953-L1076)
  • Socket::AncillaryData#family
  • Socket::AncillaryData#level
  • Socket::AncillaryData#type
  • Socket::AncillaryData#data
  • Socket::AncillaryData#cmsg_is?
  • Socket::AncillaryData#int
  • Socket::AncillaryData#unix_rights
  • Socket::AncillaryData#timestamp with the decision to not support ancillary data I'm not going to bother implementing this method as I have no idea where it would get its data from any way.
  • Socket::AncillaryData#ip_pktinfo
  • Socket::AncillaryData#ipv6_pktinfo
  • Socket::AncillaryData#ipv6_pktinfo_addr
  • Socket::AncillaryData#ipv6_pktinfo_ifindex

Specs to verify

  • spec/addrinfo/afamily_spec.rb
  • spec/addrinfo/bind_spec.rb
  • spec/addrinfo/canonname_spec.rb
  • spec/addrinfo/connect_from_spec.rb
  • spec/addrinfo/connect_spec.rb
  • spec/addrinfo/connect_to_spec.rb
  • spec/addrinfo/family_addrinfo_spec.rb
  • spec/addrinfo/foreach_spec.rb
  • spec/addrinfo/getaddrinfo_spec.rb
  • spec/addrinfo/getnameinfo_spec.rb
  • spec/addrinfo/initialize_spec.rb
  • spec/addrinfo/inspect_sockaddr_spec.rb
  • spec/addrinfo/inspect_spec.rb
  • spec/addrinfo/ip_address_spec.rb
  • spec/addrinfo/ip_port_spec.rb
  • spec/addrinfo/ip_spec.rb
  • spec/addrinfo/ip_unpack_spec.rb
  • spec/addrinfo/ipv4_loopback_spec.rb
  • spec/addrinfo/ipv4_multicast_spec.rb
  • spec/addrinfo/ipv4_private_spec.rb
  • spec/addrinfo/ipv4_p_spec.rb
  • spec/addrinfo/ipv6_linklocal_spec.rb
  • spec/addrinfo/ipv6_loopback_spec.rb
  • spec/addrinfo/ipv6_mc_global_spec.rb
  • spec/addrinfo/ipv6_mc_linklocal_spec.rb
  • spec/addrinfo/ipv6_mc_nodelocal_spec.rb
  • spec/addrinfo/ipv6_mc_orglocal_spec.rb
  • spec/addrinfo/ipv6_mc_sitelocal_spec.rb
  • spec/addrinfo/ipv6_multicast_spec.rb
  • spec/addrinfo/ipv6_p_spec.rb
  • spec/addrinfo/ipv6_sitelocal_spec.rb
  • spec/addrinfo/ipv6_to_ipv4_spec.rb
  • spec/addrinfo/ipv6_unspecified_spec.rb
  • spec/addrinfo/ipv6_v4compat_spec.rb
  • spec/addrinfo/ipv6_v4mapped_spec.rb
  • spec/addrinfo/listen_spec.rb
  • spec/addrinfo/marshal_dump_spec.rb
  • spec/addrinfo/marshal_load_spec.rb
  • spec/addrinfo/pfamily_spec.rb
  • spec/addrinfo/protocol_spec.rb
  • spec/addrinfo/socktype_spec.rb
  • spec/addrinfo/tcp_spec.rb
  • spec/addrinfo/to_sockaddr_spec.rb
  • spec/addrinfo/to_s_spec.rb
  • spec/addrinfo/udp_spec.rb
  • spec/addrinfo/unix_path_spec.rb
  • spec/addrinfo/unix_p_spec.rb
  • spec/addrinfo/unix_spec.rb
  • spec/basicsocket/close_read_spec.rb
  • spec/basicsocket/close_write_spec.rb
  • spec/basicsocket/connect_address_spec.rb
  • spec/basicsocket/do_not_reverse_lookup_spec.rb
  • spec/basicsocket/for_fd_spec.rb
  • spec/basicsocket/getpeereid_spec.rb
  • spec/basicsocket/getpeername_spec.rb
  • spec/basicsocket/getsockname_spec.rb
  • spec/basicsocket/getsockopt_spec.rb
  • spec/basicsocket/recvmsg_nonblock_spec.rb
  • spec/basicsocket/recvmsg_spec.rb
  • spec/basicsocket/recv_nonblock_spec.rb
  • spec/basicsocket/recv_spec.rb
  • spec/basicsocket/sendmsg_nonblock_spec.rb
  • spec/basicsocket/sendmsg_spec.rb
  • spec/basicsocket/send_spec.rb
  • spec/basicsocket/setsockopt_spec.rb
  • spec/basicsocket/shutdown_spec.rb
  • spec/ipsocket/addr_spec.rb
  • spec/ipsocket/getaddress_spec.rb
  • spec/ipsocket/peeraddr_spec.rb
  • spec/ipsocket/recvfrom_spec.rb
  • spec/option/int_spec.rb
  • spec/option/linger_spec.rb
  • spec/option/new_spec.rb
  • spec/socket/accept_nonblock_spec.rb
  • spec/socket/accept_spec.rb
  • spec/socket/ancillary_data/cmsg_is_p_spec.rb
  • spec/socket/ancillary_data/data_spec.rb
  • spec/socket/ancillary_data/family_spec.rb
  • spec/socket/ancillary_data/initialize_spec.rb
  • spec/socket/ancillary_data/int_spec.rb
  • spec/socket/ancillary_data/ip_pktinfo_spec.rb
  • spec/socket/ancillary_data/ipv6_pktinfo_addr_spec.rb
  • spec/socket/ancillary_data/ipv6_pktinfo_ifindex_spec.rb
  • spec/socket/ancillary_data/ipv6_pktinfo_spec.rb
  • spec/socket/ancillary_data/level_spec.rb
  • spec/socket/ancillary_data/type_spec.rb
  • spec/socket/ancillary_data/unix_rights_spec.rb
  • spec/socket/bind_spec.rb
  • spec/socket/connect_nonblock_spec.rb
  • spec/socket/connect_spec.rb
  • spec/socket/constants_spec.rb
  • spec/socket/for_fd_spec.rb
  • spec/socket/getaddrinfo_spec.rb
  • spec/socket/gethostbyaddr_spec.rb
  • spec/socket/gethostbyname_spec.rb
  • spec/socket/gethostname_spec.rb
  • spec/socket/getnameinfo_spec.rb
  • spec/socket/getservbyname_spec.rb
  • spec/socket/ip_address_list_spec.rb
  • spec/socket/ipv6only_bang_spec.rb
  • spec/socket/listen_spec.rb
  • spec/socket/local_address_spec.rb
  • spec/socket/new_spec.rb
  • spec/socket/pack_sockaddr_in_spec.rb
  • spec/socket/pack_sockaddr_un_spec.rb
  • spec/socket/pair_spec.rb
  • spec/socket/recvfrom_nonblock_spec.rb
  • spec/socket/recvfrom_spec.rb
  • spec/socket/remote_address_spec.rb
  • spec/socket/socketpair_spec.rb
  • spec/socket/socket_spec.rb
  • spec/socket/sysaccept_spec.rb
  • spec/socket/unpack_sockaddr_in_spec.rb
  • spec/socket/unpack_sockaddr_un_spec.rb
  • spec/tcpserver/accept_nonblock_spec.rb
  • spec/tcpserver/accept_spec.rb
  • spec/tcpserver/initialize_spec.rb
  • spec/tcpserver/listen_spec.rb
  • spec/tcpserver/sysaccept_spec.rb
  • spec/tcpsocket/gethostbyname_spec.rb
  • spec/tcpsocket/initialize_spec.rb
  • spec/tcpsocket/local_address_spec.rb
  • spec/tcpsocket/recv_spec.rb
  • spec/tcpsocket/remote_address_spec.rb
  • spec/udpsocket/bind_spec.rb
  • spec/udpsocket/connect_spec.rb
  • spec/udpsocket/initialize_spec.rb
  • spec/udpsocket/inspect_spec.rb
  • spec/udpsocket/local_address_spec.rb
  • spec/udpsocket/recvfrom_nonblock_spec.rb
  • spec/udpsocket/remote_address_spec.rb
  • spec/udpsocket/send_spec.rb
  • spec/unixserver/accept_nonblock_spec.rb
  • spec/unixserver/accept_spec.rb
  • spec/unixserver/initialize_spec.rb
  • spec/unixserver/listen_spec.rb
  • spec/unixserver/sysaccept_spec.rb
  • spec/unixsocket/addr_spec.rb
  • spec/unixsocket/initialize_spec.rb
  • spec/unixsocket/local_address_spec.rb
  • spec/unixsocket/path_spec.rb
  • spec/unixsocket/peeraddr_spec.rb
  • spec/unixsocket/recvfrom_spec.rb
  • spec/unixsocket/recv_io_spec.rb
  • spec/unixsocket/remote_address_spec.rb
  • spec/unixsocket/send_io_spec.rb
  • spec/unixsocket/socketpair_spec.rb

General

  • Use the right sockaddr structs for IPv4, IPv6, and Unix sockets.
  • Stop using IO::Socketable (and remove from Rubinius once no longer used)
@YorickPeterse

This comment has been minimized.

Member

YorickPeterse commented Feb 24, 2015

Note: this requires Rbx master since it adds a few new struct configurations this PR relies on.

@YorickPeterse YorickPeterse self-assigned this Feb 24, 2015

@YorickPeterse YorickPeterse changed the title from Support for Socket.ip_address_list to Support for Socket.ip_address_list and Addrinfo Feb 24, 2015

@YorickPeterse YorickPeterse force-pushed the yak-address-list branch 3 times, most recently from 092753c to 9ca7575 Feb 25, 2015

@YorickPeterse

This comment has been minimized.

Member

YorickPeterse commented Feb 27, 2015

Note to self: I'm mixing afamily and pfamily in various specs. For example, afamily indeed is AF_INET but it's pfamily that returns PF_UNSPEC by default.

@YorickPeterse

This comment has been minimized.

Member

YorickPeterse commented Feb 27, 2015

@YorickPeterse YorickPeterse force-pushed the yak-address-list branch 2 times, most recently from c6cf413 to 25fab7d Mar 3, 2015

@YorickPeterse

This comment has been minimized.

Member

YorickPeterse commented Apr 7, 2015

@brixen @jemc @sshao @jc00ke @vconcepcion Spamming all of you about this: currently I'd say about 30% of the work is done, but looking at my schedule for the coming weeks I don't think I'll have much time to continue on this. That and it's quite boring to be honest, so motivation to spend my weekends on this is rather lacking.

As such, if anybody wants to jump in and help (or finish it), please do so. Most of the work I've done so far is mainly based on MRI's behaviour as well as the RDoc documentation. I also think the above list of checkboxes is up to date with the list of commits, this should make it a little bit easier to see what remains to be done.

@jemc

This comment has been minimized.

Member

jemc commented Apr 7, 2015

I can help with some of this.
@brixen - can I get push access to this repo so I can push directly to this branch?

@YorickPeterse

This comment has been minimized.

Member

YorickPeterse commented Apr 7, 2015

@jemc I've added you to the maintainers team, which should give you enough push access.

@YorickPeterse

This comment has been minimized.

Member

YorickPeterse commented Jul 20, 2015

@digitalextremist You left a comment but removed it afterwards by the looks of it. What was it about?

@digitalextremist

This comment has been minimized.

digitalextremist commented Jul 20, 2015

Caught me. I made a mistaken assumption about Addrinfo#ip_port then caught myself. I was expecting Socket#local_address#ip_port

So I added Socket#local_address and another method I can't recall right now, but no tests yet. After that it worked as I expected. I'm trying to pull available ports by passing port 0

@YorickPeterse

This comment has been minimized.

Member

YorickPeterse commented Jul 20, 2015

Ah right, the Socket code is quite confusing mainly due to the naming of things.

@digitalextremist

This comment has been minimized.

digitalextremist commented Jul 20, 2015

@YorickPeterse yeah. That's for sure. But all the methods to connect this all together seem to be there.

Ok, now I can see my changelog:

  • Fixed: Socket.bind
  • Fixed: Addrinfo.tcp
  • Added: Addrinfo#to_sockaddr
  • Added: Socket#local_address

There are some logical easy ones caused by that, like Socket#remote_address

@brixen rekindled my interest in Rubinius so I'm making my existing applications compatible.

@YorickPeterse

This comment has been minimized.

Member

YorickPeterse commented Oct 9, 2015

TODO list for Addrinfo#initialize for the weekend:

  • Add extra tests for Addrinfo#initialize (if any are still missing) to make sure it truly behaves like MRI
  • Use a constant Array for the various if statements that check for multiple values, this constant should be scoped somewhere under RubySL::Socket
  • Use somewhat more meaningful error messages instead of just "Unsupported protocol family"
  • Re-write the descriptions of some of the auto-generated specs, right now they're a pain to read

@YorickPeterse YorickPeterse force-pushed the yak-address-list branch 2 times, most recently from e75f5d2 to 30c1e39 Oct 23, 2015

@YorickPeterse

This comment has been minimized.

Member

YorickPeterse commented Oct 26, 2015

Personal note: with some tweaking I should be able to move local_address and remote_address into the BasicSocket class so the methods only have to be implemented once. This however requires that all socket classes some expose:

  • The address family
  • The hostname/path (in case of Unix sockets)
  • The actual port number (even when binding to port 0)
  • The socket type (this can be hard-coded in the constructor method if needed)

edit: scratch that, this introduces too much crazy code. For now I'll keep the methods separately.

@YorickPeterse YorickPeterse force-pushed the yak-address-list branch from 09bc755 to 45899eb Oct 28, 2015

@YorickPeterse YorickPeterse force-pushed the yak-address-list branch from 592d938 to 2d9f58d Oct 31, 2015

@YorickPeterse YorickPeterse force-pushed the yak-address-list branch 5 times, most recently from 436c7a9 to aa2f07a Nov 23, 2015

@YorickPeterse

This comment has been minimized.

Member

YorickPeterse commented Dec 26, 2015

Fixes #11 #10 #8 #7 #6 #5 #4 #3 #1

YorickPeterse added some commits Dec 26, 2015

Fix reading data in BasicSocket#recvmsg
Due to not specifying a specific size to read sometimes the returned
data would include random garbage.
Fixed free'ing of ifaddrs structs
The C structures contain various pointers that can't be passed to
free(), most likely because they're pointers to static memory. Rubinius
in turn tries to free these pointers the moment they go out of scope as
"autorelease" is enabled by default (including for regular FFI::Pointer
instances). This in turn would lead to segmentation faults whenever the
finalizer thread would make an attempt to free() invalid memory.

To work around this the autorelease option is disabled for all pointers
stored in an Ifaddrs instance, including the pointer of the structure
itself.
Fixed reading data in recvmsg(), again
recvmsg() returns the total message size so we can use this instead of
the total buffer size (which may be larger than the returned message).
@YorickPeterse

This comment has been minimized.

Member

YorickPeterse commented Dec 26, 2015

@brixen All done, we're passing all specs. Shall I leave you to reviewing about 10000 line changes, or shall we just merge things right away? :trollface:

@YorickPeterse

This comment has been minimized.

Member

YorickPeterse commented Dec 26, 2015

FYI this also needs rubinius/rubinius@a84783d.

Updated Travis config
This enables the use of containers as well as testing on both Linux and
OS X.
@YorickPeterse

This comment has been minimized.

Member

YorickPeterse commented Dec 28, 2015

Since Travis refuses to build this pull request I'll be merging this (and praying OS X still works) some time this evening.

@YorickPeterse YorickPeterse merged commit 61f6ccc into 2.0 Dec 28, 2015

@YorickPeterse YorickPeterse deleted the yak-address-list branch Dec 28, 2015

@jc00ke

This comment has been minimized.

jc00ke commented Dec 28, 2015

Awesome work @YorickPeterse!

@brixen

This comment has been minimized.

Contributor

brixen commented Dec 30, 2015

👍

@saizai

This comment has been minimized.

saizai commented Jun 8, 2016

Which RBX versions have this update?

@YorickPeterse

This comment has been minimized.

Member

YorickPeterse commented Jun 8, 2016

@saizai This has been available since Rubinius 3.8 (rubinius/rubinius@34af1db).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment