Skip to content
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

Precompilation breaks when run as a node.js subprocess #12941

Closed
MikeInnes opened this issue Sep 4, 2015 · 20 comments
Closed

Precompilation breaks when run as a node.js subprocess #12941

MikeInnes opened this issue Sep 4, 2015 · 20 comments
Labels
bug Indicates an unexpected problem or unintended behavior compiler:precompilation Precompilation of modules io Involving the I/O subsystem: libuv, read, write, etc. priority This should be addressed urgently upstream The issue is with an upstream dependency, e.g. LLVM

Comments

@MikeInnes
Copy link
Member

Precompilation runs successfully (at least for one or two packages at a time) but leaves STDOUT and co broken:

// test.js
var child_process = require("child_process");

var process = child_process.spawn("julia", [],
                                  {stdio:'inherit'})
$ node test.js
julia> using JSON
INFO: Recompiling stale cache file /Users/mike/.julia/lib/v0.4/JSON.ji for module JSON.

Assertion failed: (r == 1), function uv__stream_osx_interrupt_select, file src/unix/stream.c, line 129.

signal (6): Abort trap: 6
__pthread_kill at /usr/lib/system/libsystem_kernel.dylib (unknown line)

If stdio is not inherited, and Julia has its own pipes, then this error doesn't kill Julia but does cause STDOUT to silently break; printing operations then fail with write: broken pipe (EPIPE).

@tkelman
Copy link
Contributor

tkelman commented Sep 5, 2015

This may be related to the issues that were being hit in #12519

@tkelman tkelman added the compiler:precompilation Precompilation of modules label Sep 5, 2015
@ihnorton ihnorton added the io Involving the I/O subsystem: libuv, read, write, etc. label Oct 2, 2015
@ViralBShah
Copy link
Member

This is preventing Juno 0.4 release, so would be nice to fix it on priority.

@ViralBShah
Copy link
Member

Bump.

@ViralBShah
Copy link
Member

@vtjnash Would be great to have your help here. We are now at 0.4.1 and don't have new Juno binaries yet - which many people are asking for.

@MikeInnes
Copy link
Member Author

So I managed to narrow this down as far as this line. Changing the redirection to, say, a file, or turning it off, fixes the issue.

Obviously there's a deeper issue with the subprocess stuff here, but perhaps we can work around this for now – is there a way to grab the STDERR stream of the subprocess directly? Then we can set up a task to redirect the output.

@JeffBezanson JeffBezanson added bug Indicates an unexpected problem or unintended behavior priority This should be addressed urgently labels Nov 24, 2015
@ViralBShah
Copy link
Member

I can't reproduce this test case on my mac on 0.4.1 or master.

@MikeInnes
Copy link
Member Author

Assuming the test case corresponds directly to the Juno issue, this does indeed appear to be resolved on the node.js side in the latest release. Unfortunately, while Electron is at node 4.1.1 (where the issue is also resolved), LT is at 1.6.3 and Atom at 2.3.1. Looking at dropping the newest electron into our LT build now.

@ViralBShah
Copy link
Member

That is fantastic. Looking forward to having Juno for Julia 0.4. People keep asking for it quite frequently!

@MikeInnes
Copy link
Member Author

Unfortunately, it looks like the original test case was exposing a different issue – I'm still seeing problems and this reproduces them.

// test.js
var child_process = require("child_process");

var proc = child_process.spawn('julia4', ['-e', 'using DataFrames; println("success")'])

proc.stdout.on('data', function(data) {
  process.stdout.write(data);
});

proc.stderr.on('data', function(data) {
  process.stderr.write(data);
});

proc.on('exit', function(data) {
  console.log('Exit: ' + data)
})
Mikes-MacBook-Pro-2:~ mike$ node test.js
INFO: Precompiling module DataFrames...
Exit: 1
Mikes-MacBook-Pro-2:~ mike$ 

@ViralBShah
Copy link
Member

Yes, I can reproduce this one now.

@ViralBShah
Copy link
Member

Definitely seems like some issue with precompiling. See how it only precompiles one dependency at a time for some reason. However, once all have been precompiled, it seems to work fine.

viral-2 09:17:14 /tmp$ node test.js 
INFO: Recompiling stale cache file /Users/viral/.julia/lib/v0.4/DataArrays.ji for module DataArrays.
Exit: 1
viral-2 09:17:27 /tmp$ node test.js 
INFO: Recompiling stale cache file /Users/viral/.julia/lib/v0.4/SortingAlgorithms.ji for module SortingAlgorithms.
Exit: 1
viral-2 09:17:36 /tmp$ node test.js 
INFO: Recompiling stale cache file /Users/viral/.julia/lib/v0.4/Docile.ji for module Docile.
Exit: 1
viral-2 09:18:21 /tmp$ node test.js 
success
Exit: 0

@vtjnash
Copy link
Sponsor Member

vtjnash commented Nov 26, 2015

the shutdown syscall isn't well documented. while you might think it merely destroys the fd, it in fact destroys the file description and all associated fd, (even across fork and dup, similar to other properties like seek position and non-blocking). we are using uv_shutdown to flush write buffers in an attempt to do a graceful blocking exit.

compare:
https://developer.apple.com/library/prerelease/mac/documentation/Darwin/Reference/ManPages/man2/shutdown.2.html
http://linux.die.net/man/2/shutdown
to a random page of a random book that actually clearly describes how it works:
https://books.google.com/books?id=2SAQAQAAQBAJ&pg=PA1343&lpg=PA1343&dq=shutdown+shut_wr+fifo+pipe&source=bl&ots=qQt5adEHty&sig=ns-JxSOyKIW-1xhTE7vSO0VD-Dg&hl=en&sa=X&ved=0ahUKEwjmg9uK8K7JAhXJXD4KHZAECi8Q6AEIHTAA#v=onepage&q=shutdown&f=false (page 1256)

i think we may need to fix this in libuv such that uv_shutdown can distinguish cases where uv_shutdown should be independent of shutdown. @Keno thoughts?

@Keno
Copy link
Member

Keno commented Nov 26, 2015

What is the condition where uv_shutdown is independent of shutdown?

@ViralBShah
Copy link
Member

For now, we can try and have Juno for 0.4 with precompilation turned off (with julia --compile=no). Given the benefits one gets from precompilation, this would definitely be a bit disappointing for users, but perhaps would at least let us get a build of Juno out.

If the shutdown issue can be fixed quickly though, we can just wait for that to happen.

@vtjnash
Copy link
Sponsor Member

vtjnash commented Nov 29, 2015

@Keno i think it is only when the stream was the inherited stdio fd

@vtjnash vtjnash added backport pending 0.4 upstream The issue is with an upstream dependency, e.g. LLVM labels Nov 29, 2015
@tkelman
Copy link
Contributor

tkelman commented Nov 29, 2015

For backporting to 0.4, note that I've made a separate julia0.4 branch of libuv, I'm concerned some of the libuv changes on master could possibly be responsible for the appveyor hangs we've been seeing. I'll probably just cherry-pick JuliaLang/libuv@030481e to that branch.

@ViralBShah
Copy link
Member

The test that @one-more-minute provided is working as expected now with this fix. Maybe the simplest thing for Juno is to make it available for download once Julia 0.4.2 is released.

@vtjnash
Copy link
Sponsor Member

vtjnash commented Nov 29, 2015

for upstreaming, we will probably need to do something better, but this'll work for our usage for now, so I wanted to just go with the minimal changes so we could get this fixed quickly

@vtjnash
Copy link
Sponsor Member

vtjnash commented Nov 29, 2015

we will also still need to do something for TCP sockets inherited as stdio, shared via IPC, or passed to a child, but that should be rare enough for now (affects all platforms)

vtjnash added a commit that referenced this issue Nov 30, 2015
using slightly different libuv branch for release-0.4
@tkelman
Copy link
Contributor

tkelman commented Dec 1, 2015

Any chance of this being responsible for https://travis-ci.org/JuliaLang/julia/jobs/94244531 (backed up as https://gist.github.com/a964717bd773af7b1ca3 for archival)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior compiler:precompilation Precompilation of modules io Involving the I/O subsystem: libuv, read, write, etc. priority This should be addressed urgently upstream The issue is with an upstream dependency, e.g. LLVM
Projects
None yet
Development

No branches or pull requests

7 participants