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

Make Go RPC work through a duplex npipe. #1

Merged
merged 2 commits into from Sep 3, 2013

Conversation

aeckleder
Copy link
Contributor

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.

@natefinch
Copy link
Owner

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.

@aeckleder
Copy link
Contributor Author

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//pull/1#issuecomment-21107120
.

@natefinch
Copy link
Owner

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.

@aeckleder
Copy link
Contributor Author

I just pushed most of your suggested changes. Please have a look at my comments for details.

natefinch added a commit that referenced this pull request Sep 3, 2013
Make Go RPC work through a duplex npipe.
@natefinch natefinch merged commit 3557f92 into natefinch:master Sep 3, 2013
xenoscopic pushed a commit to xenoscopic/npipe that referenced this pull request Nov 30, 2015
fix Accept/Close race; handle errno 0x03 on Listen; tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants