closure-compiler hangs on some javascript

[ghazel]

I'm trying to compile a js blob (which is mostly already compressed, but ignore that) but compiler.compile(blob) never returns. It does work on most blobs, but not this particular one: http://codepad.org/Jwn3rPbM

The problem seems to happen on any platform. However! This problem is fun because:

• running java directly and passing the file in with --js works
• running java directly and piping the data it (cat blob | java ...) works

Here's the really fun bit:

• closing stderr before trying to read from stdout works (but POpen4 crashes when it tries to rewind stderr)

This doesn't seem to be a POpen4 bug exactly, since the same behavior is true of Ruby's popen3, or even Python's subprocess.Popen!

I've observed this as well -- the Closure Compiler hangs (or used to, at least) on the compressed version of jQuery. My recommendation for folks who encounter this was not to double-compress the file -- once is enough, and I was hoping that a newer release of the Closure Compiler would fix the bug. Unfortunately, that doesn't seem to have happened yet.

Close stderr before reading stdout... yikes.

I think that this is more of a bug for the Closure Compiler team than it is something that we can fix ourselves. (I've already added a cautionary note to the Jammit documentation) Feel like telling them about it?

[ghazel]

My guess is that this occurs when something is written to stderr before stdout. The only case where I've found that this could happen is when warnings appear.

Sure enough, adding --warning_level QUIET to my command line makes everything work as expected.

Maybe this is a workable solution until this is fixed upstream?

[ghazel]

Ok, this is not a closure problem so it won't be fixed upstream. Here's a very tiny repro of the issue:

require 'open3'
cmd = "ruby -e 'STDERR.write(101.chr*819200);STDOUT.write(111.chr*819200)'"
stdin, stdout, stderr = Open3.popen3(cmd)
print stdout.read.length

The situation is that the stderr buffer is full, but the child process is trying to write more. The parent process is still waiting for bytes on stdout, so the whole thing hangs.

One way around this is to select on the fds and use fd.sysread. However, this doesn't work on Windows where select is only for sockets. The way Python handles this cross-platform is with threads, but ruby 1.8 threads are green threads, and are not concurrent during a blocking read.

Unfortunately using --warning_level QUIET will not help if there are real errors and the number of them exceeds the stderr buffer. This tends to happen more with already minified javascript because the warnings and errors output by closure quote the original file on the line in question, which is sometimes the entire file.

So, I don't see anything like a good answer available. Here are the possibilities I can think of:

• use --js_output_file and ignore stdout (and hope closure never writes a bunch to stdout)
• run closure once with stderr closed, a second time with stdout closed if the status code does not indicate success, to collect errors
• run python and pass the data back serialized with json or something over stdout (haha)

I spent a little while trying to get your example JS to work with read_nonblock, albeit unsucessfully. Here's the test loop I was using:

      begin
        puts 'trying'
        error += stderr.read_nonblock(1000000)
        result += stdout.read_nonblock(1000000)
      rescue Errno::EAGAIN => e
        puts error
        puts result
        sleep 0.1
        retry
      rescue Errno::EINTR, EOFError => e
        # Our job is done.
      end

There's always going to be a problem with something like this unless we can distinguish the stdout EOFError from the stderr EOFError, so it would probably best be done with two separate methods, recursively calling each other until both were finished.

Unfortunately, this still hangs -- after the big fat error is printed WARNING - Suspicious code. This code lacks side-effects. Is there a bug?, it prints "trying" one more time and then hangs indefinitely. When I interrupt the process, it's apparently stuck within waitpid2, for whatever that's worth.

I guess the alternative would be to give up on interprocess communication from within Ruby altogether, and just write out a tempfile and shell out to the Closure Compiler, no?

[ghazel]

Unfortunately read_nonblock uses fcntl to set the non-blocking flag on the file, which won't work on Windows.

Give up on the pipe to closure is one way. Since this is already going to take some time I suppose touching disk is no big deal.

[jashkenas]

Alright, I think we got this one licked. Using threads works just fine -- despite being green, the one case where you do get real concurrency with Ruby is during blocking IO. Give the latest commit a spin. If it works for you, I'll push out a new gem.

[ghazel]

No luck. I added some prints:

p 'start'
out_thread = Thread.new {
  p 'othread'
  result = stdout.read
  p 'othread end'
}
err_thread = Thread.new {
  p 'ethread'
  error  = stderr.read
  p 'ethread end'
}
p 'joining'
out_thread.join and err_thread.join
p 'done!'

and I get:

"start"
"othread"
"ethread"
"joining"
...

Fascinating -- did you use the one-click installer to compile your Ruby? Apparently certain build of Windows/Ruby have threading issues:

http://blade.nagaokaut.ac.jp/cgi-bin/scat.rb/ruby/ruby-talk/229801
http://blade.nagaokaut.ac.jp/cgi-bin/scat.rb/ruby/ruby-talk/229803
http://blade.nagaokaut.ac.jp/cgi-bin/scat.rb/ruby/ruby-talk/229829

[ghazel]

Indeed I did use the one-click installer. Hm.

[ghazel]

Wow. Ruby is just broken on win32. The comment "c) msys compiled ruby does" is not true, unless I'm missing something. Compiling with mingw still uses winsock, and so stdin is still not valid to pass to select(). There's a hack in the win32 select wrapper which says:

// assume normal files are always readable/writable
// fake read/write fd_set and return value

What it is really doing is assuming all non-sockets are readable/writable (because only sockets are valid for select). This applies to pipes like stdin instead of just regular files, so it thinks stdin is always readable, which it is not.

That's horrifying, and, sadly, not terribly surprising. I remember Luis Lavena talking about Ruby/Windows issues being "the tip of the iceberg", and trying really hard to encourage more folks to contribute. Speaking of which, do you think that hack is something that's easily patched? Or is it a more fundamental problem? I'm sure he'd love to take the patch -- and contributing to core Ruby isn't something you get to do every day.

I'm afraid we're close to out of options at this point. I'd prefer to not write tempfiles, because capturing stdout/stderr while shelling out has its own cross-platform issues. I tried to implement it with IO.select for a while, but that never managed to get around the deadlock. Let me know what your preferred plan is -- the current source is compiling your example preprocessed JS as one of the test cases, and is working fine on Mac and Linux.

[ghazel]

Ruby 1.9 has this Windows bug fixed by having real threads and a GIL, so hacking around it on 1.8.x might not have much value to people. It is possible, however, simply by having one thread per pipe internal to Ruby, for things like stdin/out/err. For pipes created by Ruby, they can use overlapped IO and run this in the select wrapper.

That's a lot of work just to get closure-compiler working for me, so I'd prefer some sort of hack. If that's a hack I have to keep local, that's fine.

Here's a select loop which should work everywhere except Windows:

fds = [stdout, stderr]
capture = {stdout => '', stderr => ''}
t = 0
while not fds.empty?
  r,w,x = select(fds)
  for fd in r
    begin
      capture[fd] += fd.sysread(1000)
    rescue EOFError
      fds.delete(fd)
    end
  end
end

[ghazel]

Here's the hack I've been using on Windows:

http://github.com/ghazel/closure-compiler/commit/dd299f3403af06869b3cff832d306c9d201b4ff8

It gives up stderr capture, and relies on a POpen fork which allows you to close stderr inside the block.

[lagrz]

It hangs on me when i pass a massive string to it as opposed to a IO stream for example if i have a 100kb js file that open and perform some operations to each individual line then try to use closure compiler by passing it that string variable it will hang.

Closing this ticket, due to the unlikely chance of getting it reliably fixed. But for the record, I just pushed a new version (0.3.0) of the gem that uses a custom version of Popen, instead of the variously problematic versions and external gem dependencies that we've tried in the past. If you'd like to take a look:

http://github.com/documentcloud/closure-compiler/blob/master/lib/closure/popen.rb

It avoids creating a grandchild process, and returns the PID of the external process to the caller, so that you can wait for it and get the correct exit code out. Hopefully this will fix the problems from before -- I'd appreciate it if someone could give it a spin on Windows.