Script runs after phantom.exit, blocks on readline

[ariya]

persianp...@gmail.com commented:

Which version of PhantomJS are you using?

1.5.0

What steps will reproduce the problem?

$ cat <<END >test.js
const fs = require('fs');
phantom.exit();
fs.open('/dev/stdin', 'r').readLine();
END
$ phantomjs test.js

What is the expected output?

The process exits, which was the behavior with 1.4.1.

What do you see instead?

The process doesn't exit until I hit CTRL-D.

Which operating system are you using?

Linux

Did you use binary PhantomJS or did you compile it from source?

source

Disclaimer:
This issue was migrated on 2013-03-15 from the project's former issue tracker on Google Code, Issue #444.
🌟   13 people had starred this issue at the time of migration.

[ariya]

persianp...@gmail.com commented:

More details:

• This only happens with /dev/stdin. Regular files and stdout seem to work fine.
• If I remove the readLine call (leaving the open call), PhantomJS still won't exit. But in this case I have to exit with CTRL-C instead of EOF.
• git bisect says 7fa1383 is the first bad commit. The good checkouts all used Qt 4.7 and the bad ones used 4.8.

[ariya]

persianp...@gmail.com commented:

Ok, sorry for the confusing report! It seems like I conflated a couple different issues, some of which are present in 1.4.1, some only in 1.5.

Also, /dev/stdin is kinda a red herring, modulo issue C. The code runs after exit the same regardless of the file. I just noticed it with stdin because the read has to wait on me.

Here's what I'm seeing:

Issue A, present in 1.4.1 and 1.5:

The script keeps running after phantom.exit. This script doesn't exit:

phantom.exit();
while (1) {}

This script does exit:

phantom.exit();
window.setTimeout('while (1) {}', 0);

Issue B, regression from 1.4.1:

PhantomJS 1.5 runs "more stuff" after phantom.exit than 1.4. This script will change myfile on disk in 1.5 but not 1.4:

f = require('fs').open('myfile', 'w');
phantom.exit();
f.write("hello\n");
f.close();

This script will exit in PhantomJS 1.4 but not 1.5:

phantom.exit();
require('webpage').create();
while (1) {}

Same thing:

f = require('fs').open('/dev/stdin', 'r');
phantom.exit();
f.readLine();

Issue C: present in 1.4.1 and 1.5, specific to /dev/stdin:

Stdin.readLine doesn't return until the file is closed. This script doesn't print "hello" when you press ENTER, only when you press CTRL-D:

require('fs').open('/dev/stdin', 'r').readLine();
console.log('hello');

Issue D: present in 1.4.1 and 1.5, not really related to my original issue:

If a script reads or writes a stream after it has been closed, PhantomJS crashes with a segfault in QIODevice::openMode():

f = require('fs').open('myfile', 'w');
f.close();
f.write('x');

Let me know if I should put any these in separate issue reports.

[ariya]

te...@gamesmademe.com commented:

I can confirm that readline (Issue C) happens also with piping

myscript.js

require('fs').open('/dev/stdin', 'r').readline()
console.log('hello')

$ (echo "1"; sleep 10; echo "2") | phantom myscript.js

Prints hello only after 10 seconds.

[ariya]

mi...@rhiza.com commented:

I'm using phantomjs 1.6.0 and can confirm that the following script will never exit:

phantom.exit()
while(1) { }

This seems really bad. I ran into this because I'm relying on a timeout function to exit my program if execution is not completed. Because exit doesn't actually stop execution it runs beyond my timeout. I can confirm that exit is being called but it doesn't seem to actually exit.

Thanks.

-bryan

[ariya]

ariya.hi...@gmail.com commented:

This non-terminating exit() has a historical baggage, it wasn't possible back then to implement it without crashing Windows.

We shall investigate again and see if this is now possible.

 
Metadata Updates

• Owner updated: alessandro.portale@gmail.com

[canonic-epicure]

nickol...@gmail.com commented:

For me - "phantom.exit()" was working correctly in 1.6.0 and is broken in 1.7.0.

[billyzkid]

billyz...@yahoo.com commented:

I'm not seeing phantom.exit working on Windows 8 with 1.7.0.

[meglio]

x.meg...@gmail.com commented:

I'm not seeing phantom.exit working on Windows 8 with 1.7.0.

The same problem for me.
Is there a workaround?

[canonic-epicure]

nickol...@gmail.com commented:

It was broken for me on Ubuntu 10.04 - seems 1.7.0 is broken in general. Workaround will be to use 1.6.0.

[ariya]

ultimus@gmail.com commented:

Ubuntu/3.7 kernel here; it's broken on 1.7.0.

Shouldn't this just map to C exit()? Surely even Windows handles that correctly.

Having a process exit function and allowing it to silently not exit just seems completely insane to me.

[ariya]

ariya.hi...@gmail.com commented:

ultimus: It's not that easy, in particular because we're running within the JavaScript world at that point. Please search past discussion or other comments in the issue tracker.

Your "insanity" comment is noted. However, if you have more constructive idea on how to solve the problem, that is more appreciated.

[ariya]

ariya.hi...@gmail.com commented:

nickolay8: What is your test case? Is it also the while(1) after exit?

[ariya]

ariya.hi...@gmail.com commented:

In some cases (but not all cases), exit has been better in 1.7.1, see issue 846.

[ariya]

ultimus@gmail.com commented:

Ariya: sorry for being direct. I just feel like this whole phantom.exit issue (at least for Linux) is kind of silly. So I cooked up a simple fix that works for my use case, and it passes 100% on the run-tests.js suite for the git master:

--- a/deps/phantomjs/src/phantom.cpp
+++ b/deps/phantomjs/src/phantom.cpp
@@ -447,7 +447,6 @@
// private:
void Phantom::doExit(int code)
{

• _Exit(code);
  if (m_config.debug())
  {
  Utils::cleanupFromDebug();

Just a forced process exit immediately on phantom.exit(). No cleanup, no waiting for anything, which is pretty much how Node.js does it. It's not pretty but it definitely works, and I don't see any practical problems with just exiting here, as far as Unix targets are concerned.

My use case is fixed with this patch so I won't push to accept it, but why not just #ifdef LINUX this for now? At least then we will have it working solid for Unix while we figure out how to get around the Windows Qt threading issues.

Because right now it looks like have the worst of all situations: this critical feature (being able to exit from the script) is broken, on all platforms, in what's supposed to be a "stable" release.

Or am I just really confused and there's some deeper issue I don't understand here?

[ariya]

ultimus@gmail.com commented:

Oh and by the way, in case anyone's wondering, this fix also corrects every Linux phantom.exit() example that I tested from this thread.

[canonic-epicure]

nickol...@gmail.com commented:

@ariya, No, my test case is more complex, can't reduce it easily.

[ariya]

ariya.hi...@gmail.com commented:

ultimus: No need to sorry about being direct, it's not an offense.

While exit immediately is a solution, care must be taken to handle different things. For example, what's the implication of abrupting the execution of JavaScript interpreter like this? Is there any other issues without terminating QApplication properly? This is far from being "silly", this is to avoid being myopic. I'm not saying that the solution is bad, it's however more than just "This 5-minute solution seems to work" and thus careful and thorough investigation is necessary.

It does not really matter how Node.js (or Ruby or Go) does it. PhantomJS has a completely different architecture.

[betelgeuse]

@ariya the semantics of the exit call are the same regardless of the architecture. So regardless of how phantomjs is built I do not follow your logic. We can rely on the kernel to clean up the resources.

[stale]

Due to our very limited maintenance capacity (see #14541 for more details), we need to prioritize our development focus on other tasks. Therefore, this issue will be automatically closed. In the future, if we see the need to attend to this issue again, then it will be reopened. Thank you for your contribution!