Closed aeckleder closed 10 years ago
Interesting. Thanks for the work. I'll look at your code. I had intended to go back and work on overlapped I/O, just never got around to it.
Hi Nate,
Did you have time to look at the code yet?
Andreas
On Wed, Jul 17, 2013 at 1:36 PM, Nate Finch notifications@github.comwrote:
Interesting. Thanks for the work. I'll look at your code. I had intended to go back and work on overlapped I/O, just never got around to it.
— Reply to this email directly or view it on GitHubhttps://github.com/natefinch/npipe/pull/1#issuecomment-21107120 .
Sorry, I hadn't gone over it very thoroughly before now. I'm looking at it now and will try to get it merged in today.
I just pushed most of your suggested changes. Please have a look at my comments for details.
The catch here is that Go RPC server continuously polls for new requests (has a blocking read going on), even while a request is running (and a reply is written back to the pipe). There are two reasons why this does not work out of the box: 1) The current os.File implementation uses a Mutex to protect the file offset it is writing to / reading from, so if a blocking Read is going on, it will keep the mutex locked. 2) Windows cannot read and write to the same file handle concurrently unless overlapped I/O is used.
Also, there is the minor issue that any broken pipe error is silently being eaten by os.File.Read().
My patch switches npipe to use ReadFile and WriteFile instead of os.File, passing an Overlapped structure to do overlapped I/O. I've also added a new test case TestGoRPC() to test Go RPC over the named pipe implementation.