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 -------------- next part -------------- An embedded and charset-unspecified text was scrubbed... Name: Socket.cs Url: http://rubyforge.org/pipermail/ironruby-core/attachments/20071229/ace5a803/attachment.pl
2007/12/29, Terence Lewis <lewistm at gmail.com>:> 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. -- Seo Sanghyeon
Terence Lewis:> 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 Messerly 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.
Terence Lewis:> 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 TCPServerThanks 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 :) )> A couple of points: > <snip> > > 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.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)> 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.Yeah, I couldn''t find any Rubinius tests for socket. JRuby might have some, I''ll need to check.> Thanks for your timeLikewise! - John
John Messerly 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...> 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.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 :) 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 Lewis:> > 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. > > 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 :)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> 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.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.> 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?Not sure whether you''re allowed to look at MRI code. We aren''t. - John
2008/1/11, Terence Lewis <lewistm at gmail.com>:> 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.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")]. -- Seo Sanghyeon
> 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.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 :) : 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
John Messerly:> > 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? > > 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