Do we care about the issue mentioned here, which concludes

If you can you should set the exit code and allow Node to exit gracefully:

process.exitCode = 1;

Nope we do not, we want to exit immediately and not just set the status code. See https://nodejs.org/api/process.html#process_process_exit_code

Btw, this issue was also known before as #395, #417, #531, #624 and also as (still open) #342.

Nope we do not,

Can you elaborate? Why do we not care? This seems to have the potential to break expected behavior, if stdout isn't flushed before exit.

This seems to have the potential to break expected behavior, if stdout isn't flushed before exit.

@flimzy, os.Exit exits the process immediately as well, it doesn't wait for buffers to flush or anything. That's the expected behavior.

Thanks for this @Lekensteyn. At a high level, this makes sense.

Does output to stdout not normally block in Go?

I think it does.

Are you referring to how writing to os.Stdout in GopherJS includes some internal buffering for browsers, so that a single line of output maps to a single line in browser console, and we might need to take that into account before exiting?

My concern, without any attempt to test this, is that something like this might lead to lost output, particularly in tests running in Node.js (which I think is only place this patch matters, right?

fmt.Println("something")
t.FailNow()

If my understanding of the SO link above, plus Go/GopherJS is correct, the Println would be translated to an async nidejs syscall, but GopherJS would treat it as if it were a completed blocking operation. So it the Node.js call may not actually complete before the program exists.

It's certainly quite possible that I'm wrong about one or more of the pieces to this puzzle, though. In that case, please accept my apologies for wasting time. :)

@flimzy Your test (or a main function which calls fmt.Println before os.Exit) does seem to print stuff correctly here. However fmt.Print("x") does not. Tested with node 8.6.0. By the time syscall(SYS_EXIT) gets executed, it is probably already too late to flush buffers (if any).

fmt.Print("x") did not work because printToConsole is doing (unconditional) line buffering. Fine for a browser, not so nice though for node. Two options: use process.stdout.write directly or invoke the internal $flushConsole call before exit.

I have just pushed one that implements the former, WDYT? The generated code is a bit ugly though,

buffer = $ifaceNil;
bufferFrom = $global.Buffer.from;
if (!(bufferFrom === undefined)) {
	buffer = new $jsObjectPtr(bufferFrom(array));
} else {
	buffer = new $jsObjectPtr(new ($global.Buffer)(array));
}
if (a1 === 1) {
	process.stdout.write($externalize(buffer, $emptyInterface));
} else {
	process.stderr.write($externalize(buffer, $emptyInterface));
}

it could also have been written as:

if (Buffer.from) {
    buffer = Buffer.from(array);
} else {
    buffer = new Buffer(array);
}
if (a1 === 1) {
    process.stdout.write(buffer);
} else {
    process.stderr.write(buffer);
}

Relevant other resources:

• stdio buffered writes (chunked) issues & process.exit() truncation nodejs/node#6456
• https://nodejs.org/api/process.html#process_a_note_on_process_i_o

@Lekensteyn Thanks for testing that.

Your approach of using process.stdout.write makes sense to me.

We should take this documented behavior into account:

In the browser, calling os.Exit (e.g. indirectly by log.Fatal) also does not terminate the execution of the program. For convenience, it calls runtime.Goexit to immediately terminate the calling goroutine.

(Source: https://github.com/gopherjs/gopherjs#application-lifecycle.)

Do we want to change that? It might break existing programs. There might've been a good reason for making that design decision, we should remember what it was.

(Or does it not apply because this change is affects node behavior only?)

The rationale for the documented behavior for browsers is noted in #229 (comment). It does not apply to node. At least, not when you run the script directly, I don't think that importing a gopherjs program as package is supported, is it?

Also note that with the syscalls package installed, os.Exit also does not return.

Ping

Thanks for the ping, and sorry about the delay. I will try to find some time to work on this.

I still need to get a better understanding of the change and gain confidence that it doesn't have any unexpected unintended side-effects.

Poke :-)

Any updates on this one?

Is any of this still relevant after the syscall rewwork

cc/ @nevkontakte

I think this is obsolete as of #1111.