Patch submission - sockets

Hello list,

Please find attached the beginnings of a sockets implementation for
IronRuby. Included in this patch is the following:

  • Class BasicSocket
  • Class IPSocket
  • Class TCPSocket
  • Class TCPServer

These classes aren’t completely fleshed out, but they are functional
to the point where I could create a very primitive web server using
TCPServer that returns a dynamic HTML page which displays the current
time, and read that page using either TCPSocket or my web browser.

A couple of points:

  1. I haven’t yet signed the contributor agreement. I did email the
    address listed on the wiki, but I haven’t heard back from it yet. I
    presume this is because of the holiday season.
  2. This is my first contribution to an open source project, and I’m
    under no illusions as to the many flaws in this code. Any comments,
    criticism or suggestion is welcome, and I assume that the code will be
    reviewed before being accepted.
  3. There are no automated tests with this code :(. I’m more than happy
    to write tests, but I need some help and guidance about what tests to
    write (or use if there are existing tests for this functionality in
    JRuby or Rubinius). It seems to me that testing sockets is quite hard,
    because of the need to make sure the tests run on different machines
    and networks.

Thanks for your time

Terence

2007/12/29, Terence L. [email protected]:

This is my first contribution to an open source project, and I’m
under no illusions as to the many flaws in this code. Any comments,
criticism or suggestion is welcome, and I assume that the code will be
reviewed before being accepted.

Hi, nice job.

  1. Mixing tabs and spaces for indentation is a bad practice.
  2. I don’t think recvfrom is correct. It calls Socket.Receive, but it
    should call Socket.ReceiveFrom.
  3. IPAddress.Any reads better than IPAddress(0).
  4. Not sure about DNS stuffs.

Terence L.:

Please find attached the beginnings of a sockets implementation for
IronRuby. Included in this patch is the following:

Hi Terrence,

Thanks for sending in this patch; John M. is reviewing it so we’ll
be in touch soon!

Thanks,
-John

PS We’ve got your contributor agreement, so that’s OK as well.

John M. wrote:

Yeah, I couldn’t find any Rubinius tests for socket. JRuby might have some, I’ll need to check.

We have some but they’re not great. Better than nothing I suppose. If
you have any additions, we’d love to get patches and additions (or port
the lot to rubyspecs and submit back).

  • Charlie

Hi John,

Thanks for taking the time to tidy up my code and check it in :). I’ve
got a few more questions…

  1. TCPSocket#open needed a different implementation. This is a thing to watch out for in singleton methods–do they return an instance of the derived type, if you call them from a derived class? For example, if you do “class MyTCPSocket < TCPSocket; end” then MyTCPSocket.open, you actually get back an instance of MyTCPSocket. The way to do this in our system is to create a dynamic site that calls “new”. Also, TCPSocket#open needed to yield to the block. I added that, which had the nice side effect of removing the need for TCPServer#open.
  1. I don’t see any “open” method anywhere in Socket.cs anymore. I
    tested it though and it still works, so I did a quick search through
    the code and came up with [RubyMethod(“open”)] in IoOps.cs. I assume
    this is what’s being called now for both TCPSocket.open and
    TCPServer.open because of BasicSocket’s inheritance from RubyIO - this
    is the dynamic site to which you refer? It is a nice side effect not
    having to handle the block :slight_smile:

  2. Regarding the ConvertToPort function (which is currently
    incorrectly spelled with only 1 t in the middle), there is a function
    in the winsock dll called getservbyname which will do this conversion
    for you - unfortunately it’s not exposed to .NET at all as far as I
    can tell. It’s also exposed directly by Ruby’s socket class as the
    method “getservbyname”, so we are going to need to call it. I’ve
    written a simple P/Invoke call which will call into ws2_32.dll to get
    this information, but I realized only after almost finishing the code
    that you guys may want to steer clear of P/Invoke for compatibility
    reasons. Is that true and if so, how can I go about implementing this
    function manually - does anybody know where I can get a list of what
    it can return? If not (i.e p/invoke is OK), does something special
    have to be done to make that code work on mono? I thought I’d get an
    answer on these questions before tidying up my code and submitting it.

  3. Can I get a quick yes/no answer as to whether I’m allowed to look
    at MRI’s source code and still contribute to this project? Also, are
    you guys (Microsoft employees) allowed to look at this code - and if
    you’re not, can I still refer to MRI source code in questions I post
    to this list regarding implementation details?

Thanks

Terence

Terence L.:

Please find attached the beginnings of a sockets implementation for
IronRuby. Included in this patch is the following:

  • Class BasicSocket
  • Class IPSocket
  • Class TCPSocket
  • Class TCPServer

Thanks for the patch. I’m checking it in now; it’ll be included in the
next SVN update.

These classes aren’t completely fleshed out, but they are functional
to the point where I could create a very primitive web server using
TCPServer that returns a dynamic HTML page which displays the current
time, and read that page using either TCPSocket or my web browser.

I created a very simple manual test written in Ruby (will be under
tests/libraries/socket/manual_socket.rb). It’s really basic, hopefully
we can expand it into an better test.

(although, it was pretty cool seeing IronRuby serve up a web page :slight_smile: )

A couple of points:

  1.  This is my first contribution to an open source project, and
    

I’m
under no illusions as to the many flaws in this code. Any comments,
criticism or suggestion is welcome, and I assume that the code will be
reviewed before being accepted.

Overall, it looks great. I made some small changes before checking in.
Basically:

  1. Added a method to convert AddressFamily values into strings (you’re
    right, it doesn’t appear System.Net has anything like that)

  2. Added Protocols.CastToString in a few places & improved conversions.
    Protocol handling in Ruby is tricky–I usually test methods out in MRI
    and see what conversion methods they call if passed an arbitrary object.
    to_str & to_int being the common ones. I think we have a Protocol method
    for most of the MRI conversions now, so typically you should just need
    to call one of them.

  3. TCPSocket#open needed a different implementation. This is a thing to
    watch out for in singleton methods–do they return an instance of the
    derived type, if you call them from a derived class? For example, if you
    do “class MyTCPSocket < TCPSocket; end” then MyTCPSocket.open, you
    actually get back an instance of MyTCPSocket. The way to do this in our
    system is to create a dynamic site that calls “new”. Also,
    TCPSocket#open needed to yield to the block. I added that, which had the
    nice side effect of removing the need for TCPServer#open.

  4. For TCPServer#new, we support optional parameters in the middle of
    the argument list, so I changed the code to use that. Also changed it to
    just take two args. Usually you don’t need a params array unless the
    method takes an arbitrary number of args.

  5. I added some to/from byte array helpers to MutableString to make
    those conversions easier. (Should get even easier once we convert to a
    byte-array based string like Ruby uses)

  1.  There are no automated tests with this code :(. I'm more than
    

happy
to write tests, but I need some help and guidance about what tests to
write (or use if there are existing tests for this functionality in
JRuby or Rubinius). It seems to me that testing sockets is quite hard,
because of the need to make sure the tests run on different machines
and networks.

Yeah, I couldn’t find any Rubinius tests for socket. JRuby might have
some, I’ll need to check.

Thanks for your time

Likewise!

  • John

2008/1/11, Terence L. [email protected]:

function manually - does anybody know where I can get a list of what
it can return? If not (i.e p/invoke is OK), does something special
have to be done to make that code work on mono? I thought I’d get an
answer on these questions before tidying up my code and submitting it.

In theory, getservbyname returns IANA-specified port numbers.
http://www.iana.org/assignments/port-numbers

In practice, implementing common ports like FTP and HTTP would suffice.

Re: P/Invoke. You need to specify the different DLL name, otherwise
it’s same. In this case, socket functions are in libc, So
[DllImport(“libc”)].

Terence L.:

tested it though and it still works, so I did a quick search through
the code and came up with [RubyMethod(“open”)] in IoOps.cs. I assume
this is what’s being called now for both TCPSocket.open and
TCPServer.open because of BasicSocket’s inheritance from RubyIO - this
is the dynamic site to which you refer? It is a nice side effect not
having to handle the block :slight_smile:

Yup, there’s just IO#open. I think it’s that way in MRI:

[BasicSocket,IPSocket,TCPSocket,TCPServer].any? { |x|
x.methods(false).include? “open” } # => false

IO.methods(false).include? “open” # => true

manually - does anybody know where I can get a list of what it can
return? If not (i.e p/invoke is OK), does something special have to be
done to make that code work on mono? I thought I’d get an answer on
these questions before tidying up my code and submitting it.

Yup, we’re trying to avoid pinvokes. Would be better if we could find a
managed way to do it. Unfortunately this post suggests there isn’t a
nice API:
http://forums.microsoft.com/MSDN/ShowPost.aspx?PostID=347426&SiteID=1.

  1. Can I get a quick yes/no answer as to whether I’m allowed to look at
    MRI’s source code and still contribute to this project? Also, are you
    guys (Microsoft employees) allowed to look at this code - and if you’re
    not, can I still refer to MRI source code in questions I post to this
    list regarding implementation details?

Not sure whether you’re allowed to look at MRI code. We aren’t.

  • John

John M.:

  1. Can I get a quick yes/no answer as to whether I’m allowed to look
    at MRI’s source code and still contribute to this project? Also, are
    you guys (Microsoft employees) allowed to look at this code - and if
    you’re not, can I still refer to MRI source code in questions I post
    to this list regarding implementation details?

Not sure whether you’re allowed to look at MRI code. We aren’t.

Got clarification. Basically, the only requirement for contributors is
to follow the contributor agreement. Otherwise, look at anything you
want.

(Also referring to MRI code on the list is fine too–people do it on
ruby-core, to which I’m subscribed, I don’t see how that would be any
different from this list)

  • John

In theory, getservbyname returns IANA-specified port numbers.
Service Name and Transport Protocol Port Number Registry

In practice, implementing common ports like FTP and HTTP would suffice.

Cool - will leave it as is for now.

On a slightly different tack then, there is the following comment in the
code:

// TODO: this won’t work at all with the existing RubyIO
implementation until I implement a proxy class that wraps Socket in a
Stream
// TODO: i think that stream should do all of the IO mode checking and
throw where appropriate … in the case of sockets, only
// the socket class knows whether it can read or write from a given
socket - the IOMode flags don’t exist at all …

I tried to use the System.IO.NetworkStream class to pass a stream
object through to RubyIO (John - was this what you meant by the above
comment?), but I ran into a few problems - notably:

  1. RubyIO calls IsEndOfStream which tries to do a peek, which in turn
    tries to get the stream’s position - NetworkStream throws an exception
    when Position is called. As a temporary solution to work around this I
    made IsEndOfStream virtual and overrode it in BasicSocket to always
    return false, which lead me onto…
  2. I could then read data off the socket via the NetworkStream in
    RubyIOOps.ReadLine, but as soon as I hit ‘\r’ GetChar() again does a
    peek to see if a ‘\n’ is coming and NetworkStream once again throws an
    exception.

These hacks on my part lead me to believe that you do not intend to
use NetworkStream directly, but rather a custom stream implementation
that will behave itself properly according to how RubyIO uses the
System.IO.Stream interface. Is that correct? If so, I’m going to try
and make a start on that custom stream (I’ll look at ConsoleStream for
insipration) - but in that case I have a few more questions (for now
:slight_smile: : For peek to work properly, I assume I’m going to have to read
data off the socket and buffer it inside the stream-socket-wrapper
class - is that correct? If so, I suppose the stream’s Position
property will have to not throw an exception (else we’re no better off
than NetworkStream when it comes to peeking), but what should it do
instead? Should it just be a do-nothing getter and setter to satisfy
RubyIO’s peek?

Thanks for your patience

Terence