ReadFile is stream-blocking on windows

[Keno]

For some reason jl_is_pty (or more precisely the pNtQueryInformationFile in uv_get socketname) blocks until another character is received on stdin.

[Keno]

I think this might also be the cause of #7082 cc @vtjnash @tkelman. As far as I can tell there is nothing wrong with the call, but I think the pipe might be locked kernel side by the read request.

[vtjnash]

that sounds about right then. perhaps we need to cache this information?

although, why would windows ever make this ever a blocking call?

[Keno]

I don't know, but looking at the ReactOS implementation of the same call [1] it does need to lock kernel objects and it is certainly possible that windows doesn't release some of these locks in the read request.

[1] http://doxygen.reactos.org/d5/de1/iofunc_8c_a1b15fffe12d8eb6777ecfb93572051ab.html

[Keno]

Looks like the problem is that this is a nonoverlapped file, hence libuv is executing a blocking ReadFile on the thread pool which holds the kernel object lock.

[Keno]

Looks like a simple

  if (handle->flags & UV_HANDLE_NON_OVERLAPPED_PIPE) {
      CancelIoEx(handle->handle, NULL);
  }

in uv_get_socketname fixes this.

[vtjnash]

ooo, nice. can you merge that locally & submit a fix upstream? this will fix one of the issues in #6795

edit: although it would be nice if this was first fixed to not cause a segfault after running a command in the REPL

[vtjnash]

as it turns out, this it incredibly non-trivial to fix. for our usage, we can simply move jl_is_pty into libuv, and call it once at startup. however, for the general case, we need to implement full locking around the libuv ReadFile and NtQueryInformationFile calls to handle passing the file lock from our read thread to our request thread and back.

[vtjnash]

since the microsoft bug reporter closed this issue as a design feature (in 2008), I'm bumping this out of 0.3, since the workaround (http://www.codeproject.com/Articles/18975/Listing-Used-Files :: ExtractAndInstallDrv) is completely absurd

[ihnorton]

For future reference, and just to go ahead-full with the absurdity:
https://code.google.com/p/mintty/issues/detail?id=56

(tl;dr: mintty has issues with "real" console applications)

[tkelman]

We seem to have had a regression with mintty here recently. Looking into it.

[vtjnash]

following #11554, is there now a call to raw! that doesn't first call stop_reading? we should probably just have raw! also have a pair of stop_reading/start_reading calls for windows.

[tkelman]

So partially undo 5863b48 in the @windows_only case? Will try that.

[vtjnash]

or just add an explicit Base.stop_reading/Base.start_reading inside the raw! function (windows only). that may be easier than reverting that commit (although either may be good for testing / bisecting)

[tkelman]

I just meant a piece of it, presumably only the changes to base/REPL.jl or base/LineEdit.jl matter here. Or I could go find where raw! is defined and try that too.

[tkelman]

Do you mean like

diff --git a/base/Terminals.jl b/base/Terminals.jl
index a62e4b8..b1dbda0 100644
--- a/base/Terminals.jl
+++ b/base/Terminals.jl
@@ -136,18 +136,21 @@ cmove_col(t::UnixTerminal, n) = write(t.out_stream, "$(CSI)$(n)G")
 end
 @windows ? begin
     function raw!(t::TTYTerminal,raw::Bool)
+        stop_reading(t)
         if ispty(t.in_stream)
             run(if raw
                     `stty raw -echo onlcr -ocrnl opost`
                 else
                     `stty sane`
                 end,t.in_stream,t.out_stream,t.err_stream)
-            true
+            ret = true
         else
-            ccall(:jl_tty_set_mode,
+            ret = ccall(:jl_tty_set_mode,
                  Int32, (Ptr{Void},Int32),
                  t.in_stream.handle, raw) != -1
         end
+        start_reading(t)
+        return ret
     end
 end : begin
     raw!(t::TTYTerminal, raw::Bool) = ccall(:uv_tty_set_mode,

? I tried that but it didn't seem to help.

[vtjnash]

yeah, that's what i was thinking

[tkelman]

Okay bisect confirms 5863b48 is at fault, any idea for other things to try? Other portions of that commit that may need partial windows-only reversion that I may have missed?

edit: still looking into it but this may have just been fixed by the timers pr some recent changes have made this not as bad, but still intermittent

[tkelman]

I'm seeing the repl test occasionally freeze when I run it from outside mintty, might be related?

[tkelman]

@vtjnash had a very good guess here: while frozen waiting for another input, stty.exe is running.

[Keno]

We should probably just rewrite the relevant parts of stty ourselves then.

[vtjnash]

tkelmen's test proves that it is the fault of julia's REPL, not stty.

[vtjnash]

synchronization mutexes is really not fun :(.

the original fix for this had a race condition if uv_read_stop was called shortly after uv_read_start or a successful read, and before the uv_pipe_zero_readfile_thread_proc thread started. There needs to be one additional check that the pipe is still active before committing to the blocking read after setting up the thread handle mutex.

i believe this is fixed by JuliaLang/libuv@bd8cf2a?w=1

[tkelman]

Yes, from initial testing it looks like that fixes the issue, thanks!

(?w=1 sure is useful)