-
Notifications
You must be signed in to change notification settings - Fork 210
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
Kernel Panic on OSX #33
Comments
Yikes. I can't think of anything that was changed recently that would have radically affected the unix/osx implementation. If pty.js was causing panics, I imagine this issue would have been posted a while ago. @javruben, what happens when you use an earlier version of node. Say version 0.8.x? (Assuming you're using v0.10.x) (Also, this might be a stupid question, but do you get the same results with a real process path and a real cwd? cc @TooTallNate |
I tried it in node 0.6.19 and node 0.8.17. Both show the same behavior. It also crashes with a real process, but it's harder to reproduce. It definitely happens during the exit phase. Often it panics when I ctrl-c the node process. |
Just tried it with this example:
And it crashes all the same. |
fyi, I had pty.js crashing both with v0.1.5 and 0.2.2. So I'm guessing this is not something new. But perhaps its something unique to my system? Could it be a dependency? |
Also, important to note is that it doesn't crash on the first execution of pty.spawn, but after N executions. N being some tenfold. |
I just tried it with node 0.8.22 and it doesn't crash anymore. |
I can't really debug this myself since I don't own a machine with OSX on it. What we can do is try eliminating the non-essentials to see what we can rule out. I can only really throw some ideas out and see if they work. The fact that it doesn't seem to panic at any consistent point makes it hard to pinpoint though. Try changing line 427 in src/unix/pty.cc. This is the most osx-specific code here. diff --git a/src/unix/pty.cc b/src/unix/pty.cc
index 97b55c7..02bd0e7 100644
--- a/src/unix/pty.cc
+++ b/src/unix/pty.cc
@@ -424,7 +424,7 @@ pty_getproc(int fd, char *tty) {
return buf;
}
-#elif defined(__APPLE__)
+#elif 0
static char *
pty_getproc(int fd, char *tty) { This will remove the process name polling code, which is totally non-essential. (Although, this code is used by tmux as well, and I don't think that panics on OSX). You may not be using this if you're just writing a simple script which does not access The fact that you say it panics on the exit of the process makes me wonder. Try commenting line 288 of lib/pty.js. That has been a part of pty.js since the beginning, and I remember a long time ago, when I first released it, there was one report of a kernel panic on OSX. diff --git a/lib/pty.js b/lib/pty.js
index 85a084c..e33469d 100644
--- a/lib/pty.js
+++ b/lib/pty.js
@@ -285,7 +285,7 @@ Terminal.prototype.destroy = function() {
// node stops reading a dead file descriptor.
// Then we can safely SIGTERM the shell.
this.socket.once('close', function() {
- self.kill('SIGTERM');
+ //self.kill('SIGTERM');
});
this.socket.destroy(); |
Very strange. Although I hate to blame node since this is really OSX's fault, I suspected it might have something to do with changes in node itself since pty.js hasn't ever been that fundamentally changed since the beginning, and because this is the first panic report I've heard in a long time. |
ok, so part of the problem was fixed by compiling pty.js for the specific node versions I tried. So now it only crashes on exit of the node process, not while running it. Will try the experiments you outlined above. |
I tried not doing the SIGTERM but it is still crashing. Note that most of the time I'm testing using node-webkit, although the behavior I previously reported was using plain node. |
I tried the line 427 change. Still same thing. I'll get some confirmation on this on a standalone node on different systems on Monday. |
Yeah, the 427 thing probably wouldn't do anything. The only thing I can't think of here is to keep trying to rule things out, maybe strip down the code to its bare essentials. I'm not sure, I don't usually debug kernel panics. I have to say I'm still very surprised this is the only kernel panic report posted so far. |
I haven't gotten around to this yet, but I will asap. Btw, where are you located? |
I just got this confirmed by a colleague using OSX. Is there any way you can help debugging this? |
I haven't seen this for a while anymore. Apparently it only happened in a specific version of OSX. Closing for now. |
Interesting. Good to know this isn't affecting all OSX users. Probably also explains why there was only one report of this after many months of that code being published to npm (not to mention I usually had at least one OSX user quickly test it for me before releasing it each time). |
Reopening. I'm wondering if this has to do with node-webkit. Crashes are still happening regularly when pty.js is loaded in node-webkit (OX 10.7.5). |
I'm also getting panics using node-webkit (0.8.6) on OSX (10.9.5). Not always but some times. Could it have something todo with the zombie processes (#58)? When Im running my unit test I spawn and destroy a lot of bash processes. Then suddenly my computer crashes... |
I have the exact issue. I'm running OSX 10.9.5 and node-webkit 0.8.6, since pty.js doesn't work with node-webkit 0.10.5 (node.js 0.11.x) yet (#88). I think the issue is happening somewhere when the pty process terminates (not all the times, seems to be quite random...) |
I'm using OSX 10.7.5. I tried both the NPM version and the latest master (26th of April).
I'm getting highly reproducible Kernel Panics. This wouldn't be a big issue, except that we're trying to release a version of Cloud9 IDE that runs on OSX.
I've listed the Kernel Panic logs here: https://gist.github.com/javruben/5472500
Here's a small app that will crash:
I noticed that when the data event is not set, it doesn't seem to crash. I didn't thoroughly test that,so I'm not sure if that is absolutely true.
It would be awesome if you could help getting this fixed.
The text was updated successfully, but these errors were encountered: