Make compatible with io.js

[snostorm]

../node_modules/nan/nan.h:409:19: error: no type named 'ExternalAsciiStringResource' in 'v8::String'; did you mean 'ExternalStringResource'?

io.js ships a newer V8 version where String::ExternalAsciiStringResource
has been replaced with String::ExternalOneByteStringResource

via nodejs/nan#223 via nodejs/node#386

[snostorm]

Whoops, I misread the logs. This is actually failing on nan.h. I'll keep this open incase the dependency needs a bump, but the semver here seem loose enough that it shouldn't be an issue.

[snostorm]

Okay, so nan v1.5 is out (and v1.4 is going to get a patch for io.js support as well) but them semver on pty.js is locking the project to v1.0.0 <= v0.1.1.

Updating this request to: can we get a semver bump on this dependency please?

[timoxley]

nodejs/node#456

[seiyria]

@snostorm would a semver bump be the only thing required to fix this issue?

[snostorm]

I think that was all that was needed. I'll need to re-test to be sure. If it was that simple ... not sure why I didn't just PR, sorry.

[snostorm]

I think this will depend on nodejs/nan#265 to land -- the latest nan+iojs I have seems to hit the issue, which builds when built against the fix living via https://github.com/touhou-gensokyo/nan.git#master --

But the npm test still fails after a "successful" compile

[chjj]

New pty.js released using nan v1.6.2. Hopefully this fixes something.

[alem0lars]

I'm still getting the error..

[hmalphettes]

@chjj it builds with node-0.12 and iojs but the tests are all failing: timing out.

[dannydulai]

the example from the readme doesnt work with node-0.12.0

it doesnt launch the right program, and prints out that it is running node.

[javruben]

@chjj We'd love to have node-0.12 support. Is there anything we can do to help?

[hmalphettes]

There is also @Gottox rewrite of pty.js: https://github.com/Gottox/child_pty
I am no expert here; I only checked out child_pty and ran the tests under node-0.10 iojs and node-0.12.
Maybe @chjj and @Gottox could give us a hint as to what direction we should take?

[Gottox]

Pro pty.js

pty.js is better tested in production and actually used in more than just demo applications. Also debugging on MacOS is hard for me, as I haven't access to a MacOS System.

pty.js has windows support.

Pro child_pty:

As child_pty does not support windows, it can be much simpler and smaller. It borrows most of it's functionality and API from nodes core module child_process, hence the name. The whole projects is smaller than 150 lines of code, not counting the tests.

Furthermore child_pty does not shim the Stream objects as pty.js does. This implicates some bugs in pty.js that this has unexpected values in some cases.

[hmalphettes]

@Gottox much appreciated! I had not realised child_pty does not support windows. I am on a mac.

[javruben]

@chjj We're willing to help get pty.js compile for node 0.12 and iojs, but wouldn't want to end up having the effort done twice. Could you help coordinate?

[chjj]

Yeah, I first noticed the issue with v0.12 a little while ago. I spent a day trying to figure out what was wrong. It looks like the pty FDs just aren't getting polled or read from properly. I'm not sure there's an easy fix from the node side.

Originally, pty.js used fs.ReadStream and fs.WriteStream to read and write to the tty FDs. That turned out to cause thread pool exhaustion. We switched to using net.Socket. That also broke with a later release of node which disallowed tty FDs being used by net.Socket. We switched to using tty.ReadStream, which seemed appropriate. This also failed.

I can play around with it a bit more to see what I get working.

[chjj]

@javruben, also, it could be that fs.ReadStream and fs.WriteStream still work, but it may cause thread pool exhaustion still. I'll try to whip something up.

[chjj]

Also, a thing to note: if we switch to using a different mechanism, we won't be able to support older versions of node without including a bit of compatibility code. I really wish this didn't change so frequently.

[chjj]

After a minute or two of fumbling around, it looks like fs.ReadStream works for the tty fd, but fs.WriteStream does not. Very confusing. Even so, these may still exhaust the thread pool. We'll have to check.

[chjj]

I actually meant to address to @hmalphettes' concern with all that.

This build issue looks like a problem with nan.h, but I still don't have issues on linux. I'll try to reproduce it on OSX again.

[clov3r]

@chjj, that commit builds and test fine on OSX 10.10.2 here. The readme example also works.

Downstream, webBoxio/atom-term2#90 is still giving the same error from the OP, but that may be a caching issue with atom / apm / npm, as that project's package.json points to master. Or maybe it needs a version bmp here? Not really sure. Maybe you or someone else here can shed some light on the issue there, maybe you can't. Either way, thanks for the time you put in.

$ apm install term2

# ...
npm ERR! Failed at the [email protected] install script 'node-gyp rebuild'.
npm ERR! This is most likely a problem with the pty.js package,
npm ERR! not with npm itself.
npm ERR! Tell the author that this fails on your system:
npm ERR!     node-gyp rebuild
npm ERR! You can get their info via:
npm ERR!     npm owner ls pty.js
npm ERR! There is likely additional logging output above.

# (full log available on request ;)

[Unitech]

IT WORKS! :D Thanks so much @chjj

[nightwing]

@chjj polling ReadStream helps to display some output, but unfortunately it introduces new issues:

var term = pty.spawn('bash', [], {
  name: 'xterm-color',
  cols: 80,
  rows: 30,
  cwd: process.env.HOME,
  env: process.env
});

term.on('error', function(err) {
  console.log("[error]", err);
});

• displays errors { [Error: EAGAIN, read] errno: -11, code: 'EAGAIN' } every time interval function is called.
• piping doesn't work

process.stdin.pipe(term)
term.pipe(process.stdout) // used to work with 0.10

• the cpu usage is too high
• polling interval is not cleared when terminal is closed

Strangely https://github.com/Gottox/child_pty works without polling so maybe the error is caused by a change in node steams?

[chjj]

@nightwing, odd. EAGAIN should be filtered out. I'll look into piping. The CPU usage was 1-2% higher for me due to the interval. I'll probably just implement the old fs.ReadStream (before it was refactored like crazy). That was a bit easier to work with and should suit our needs better.

child_pty seems to do the exact same thing. pty.js originally used the FS streams in the early days. I don't see why it wouldn't work now. I'll have to investigate. It could be that child_pty does not use non-blocking file descriptors, which means child_pty would severely fail if you get more than a few pty's open (assuming node/libuv handles the FDs the same way).

[chjj]

@nightwing, my suspicions were confirmed. The FS read stream works for child_pty because it doesn't set the tty FDs to nonblocking. That's bad. It should not be using blocking FDs. We can't settle for that. It causes strain on the thread pool (a thread necessary for each tty FD), eventually leading to thread pool exhaustion. I'll be including my own read stream so we don't need the interval.

[chjj]

@nightwing, here's a first pass at using our own ReadStream. Ideally we would implement our own IO watcher like node.js used to have to avoid unnecessary reads, but it's better than before, and definitely better than using blocking FDs.

[chjj]

I'll have to dig into node and libuv to see what became of IO watchers (as they were called in old versions of node.js). That's all I'm familiar with for polling nonblock FDs in node.js. If anyone else has knowledge on the subject that would be good.

[chjj]

@nightwing, try the above commit. This might be a better fix. It simply wraps net.Socket and forces it to handle tty FDs like pipes (which is, in a way what they are). However, this might not work on all OSes. It would be nice to get people to test it.

This saves us a lot of trouble if the problem with using tty fds with net.Socket is negligible. I suppose we could research old node.js commits and find out.

[nightwing]

@chjj awesome! This works well on ubuntu.

[chjj]

Good. Works on Arch and OSX too.