Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Allow the code to decide if it should use read or read2. #3674

Closed
wants to merge 1 commit into from

5 participants

@VanCoding

This change makes the whole thing a bit more dynamic & it allows one to receive handles from pipes that we got from a pipe server. Hope you pull it.

@bnoordhuis bnoordhuis commented on the diff
src/stream_wrap.cc
@@ -132,8 +132,8 @@ void StreamWrap::UpdateWriteQueueSize() {
UNWRAP(StreamWrap)
- bool ipc_pipe = wrap->stream_->type == UV_NAMED_PIPE &&
- ((uv_pipe_t*)wrap->stream_)->ipc;
+ bool ipc_pipe = (args.Length()>0)?(args[0].IsTrue()):(wrap->stream_->type == UV_NAMED_PIPE &&
+ ((uv_pipe_t*)wrap->stream_)->ipc);
@bnoordhuis Owner

One, this is unreadable. Two, what do you think happens when you call uv_read2() on a stream type that doesn't support it?

I can of course make it more readable & handle errors, too. But only if you are willing to pull it then ;)

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

it allows one to receive handles from pipes that we got from a pipe server

That will not work. Maybe on Unix.

@VanCoding

On unix, it definitely works, but I don't know about Windows. I'll check that ;)

@VanCoding

You're right @piscisaureus, it doesn't work on windows. But it works well on linux.

What's the problem on windows? Is windows not able to do it or is it just not implemented at the moment? Since handle passing over pipes works in the cluster module, it should also work with named pipes, doesn't it?

Anyway, I tried to do this as a simple workaround for a bug in libuv. Libuv doesn't set the "ipc" flag for pipe handles gotten from a pipe server handle. This results in the problem, that node also doesn't call read2 on them and its not possible to pass handles over such pipes then.

I've already asked to fix this in the following issue: joyent/libuv#490

If you plan to fix this soon anyway, then this pull request will become useless.

@VanCoding

As a proof that it works, you can look at the latest version of my module "node-ancillary" at https://github.com/VanCoding/node-ancillary. It has a native funciton "makeIPC" which does nothing more that setting the ipc property of uv_pipe_t handles to 1. This allows the whole module to do its job.

I hope there will be a way to do the same on windows soon.

@piscisaureus

@VanCoding On windows there is no ancillary data for pipes. So IPC channels implement their own special protocol that only libuv understands. The thing is, there is no way to libuv to tell whether a pipe uses this IPC protocol, or is just "raw". When node spawns a child process with fork() it sets some environment variable so the child node knows which stdio handle uses the IPC protocol. For pipe servers there's not really a way to do this.

@VanCoding

Ok, good to know. But just to be sure: Since you use a protocol to exchange handles, it should theoretically work with every binary stream. It would even work with tcp sockets, right?

The only thing I'd have to do is to tell libuv that it shall use this ipc protocol for the stream. The only problem at the moment is that libuv doesn't know if it should use this protocol or just the raw stream for pipes it got from pipe servers.

If this is true, then I should still be able to manually make such a pipe use the ipc protocol. But how can I do it?

One more thing: No matther if it's a normal pipe or a pipe server, we can set the "ipc" property for both of them. Why don't you automatically use the ipc protocol for all pipes gotten from a pipe server whose ipc-property is set to true? It would make a lot of sense in my opinion. Again a linkt to my feature request at libuv: joyent/libuv#490

@Nodejs-Jenkins

Can one of the admins verify this patch?

@chrisdickinson

Closing this due to inactivity.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 2 additions and 2 deletions.
  1. +2 −2 src/stream_wrap.cc
View
4 src/stream_wrap.cc
@@ -132,8 +132,8 @@ Handle<Value> StreamWrap::ReadStart(const Arguments& args) {
UNWRAP(StreamWrap)
- bool ipc_pipe = wrap->stream_->type == UV_NAMED_PIPE &&
- ((uv_pipe_t*)wrap->stream_)->ipc;
+ bool ipc_pipe = (args.Length()>0)?(args[0].IsTrue()):(wrap->stream_->type == UV_NAMED_PIPE &&
+ ((uv_pipe_t*)wrap->stream_)->ipc);
@bnoordhuis Owner

One, this is unreadable. Two, what do you think happens when you call uv_read2() on a stream type that doesn't support it?

I can of course make it more readable & handle errors, too. But only if you are willing to pull it then ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
int r;
if (ipc_pipe) {
r = uv_read2_start(wrap->stream_, OnAlloc, OnRead2);
Something went wrong with that request. Please try again.