crystal-lang / crystal

The Crystal Programming Language
https://crystal-lang.org
Apache License 2.0
19.5k stars 1.62k forks source link

Console streams are blocking on Windows #14576

Open straight-shoota opened 6 months ago

straight-shoota commented 6 months ago

This has been mentioned before but I don't think there's a dedicated issue for it.

The standard streams on Windows are blocking. On Unix systems, on the other hand, they are non-blocking for TTYs. It allows to continue other fibers while waiting on IO. This is particularly relevant for STDIN.

For example, the following program prints the current time and updates it every second, while pressing enter aborts.

spawn do
  loop do
    print Time.utc.to_s("%H:%M:%S\r")
    sleep 1.second
  end
end

STDIN.gets

On Windows, it doesn't print anything because STDIN.gets is blocking and the loop fiber never gets a chance to execute.

We should have the same behaviour as on Unix. I'm not sure how we can best achieve it.

HertzDevil commented 6 months ago

IIRC Win32 console handles do not support overlapped I/O, so any kind of asynchronous capability requires threads; the standard input and output streams are not opened with FILE_FLAG_OVERLAPPED, and additional console handles opened using CreateFile do not respect that flag

HertzDevil commented 3 months ago

This snippet echoes the standard input while printing an asterisk every second in the background, using a dedicated thread for reading from STDIN. It should work with or without -Dpreview_mt, and with both redirected or console input:

require "colorize"

lib LibC
  fun PostQueuedCompletionStatus(completionPort : HANDLE, dwNumberOfBytesTransferred : DWORD, dwCompletionKey : ULONG_PTR, lpOverlapped : OVERLAPPED*) : BOOL
end

module AsyncStdin
  @@read_cv = Thread::ConditionVariable.new
  @@read_mtx = Thread::Mutex.new

  @@iocp : LibC::HANDLE = LibC::INVALID_HANDLE_VALUE
  @@read_key = Crystal::IOCP::CompletionKey.new

  @@done_cv = Thread::ConditionVariable.new
  @@done_mtx = Thread::Mutex.new

  @@buf : String?

  @@stdin_read = Thread.new do
    while true
      @@read_mtx.synchronize do
        until @@read_key.fiber
          @@read_cv.wait(@@read_mtx)
        end
      end

      # this shall be baked into `ConsoleUtils.read_console` instead, which also implies
      # this entire thread shouldn't need to interact with any Crystal scheduler directly
      @@buf = STDIN.gets.not_nil!

      # `JOB_OBJECT_MSG_EXIT_PROCESS` is to reuse the resumption mechanism for `Process.run`
      LibC.PostQueuedCompletionStatus(@@iocp, LibC::JOB_OBJECT_MSG_EXIT_PROCESS, @@read_key.object_id, nil)

      @@done_mtx.synchronize do
        while @@buf
          @@done_cv.wait(@@done_mtx)
        end
      end
    end
  end

  def self.read_stdin
    @@read_mtx.synchronize do
      # TODO: what happens if there are concurrent reads? replace these with a queue?
      @@iocp = Crystal::EventLoop.current.iocp
      @@read_key.fiber = Fiber.current
      @@read_cv.signal
    end

    Fiber.suspend

    @@done_mtx.synchronize do
      buf, @@buf = @@buf, nil
      @@done_cv.signal
      buf
    end
  end

  spawn do
    while true
      print '*'.colorize.magenta
      sleep 1
    end
  end

  while true
    print read_stdin.colorize.green
  end
end
HertzDevil commented 2 months ago

I'm sure STDOUT and STDERR can be made non-blocking in a similar manner, but what differences would that make?

straight-shoota commented 2 months ago

Considering that the synchronisation happens entirely inside the Crystal runtime, I'm wondering if we even need to go through IOCP in order to signal completion. Instead we could enqueue a resume event directly into the event loop.

So something like this would do as well (maybe could replace Channel with a lower-level synchronization primitive, but it should handle concurrent reads well):

  @@read_requests = Channel({Pointer(String?), Fiber}).new

  @@stdin_read = Thread.new do
    while request = @@read_requests.receive?
      buff_pointer, fiber = request
      buff_pointer.value = STDIN.gets
      fiber.enqueue
    end
  end

  def self.read_stdin
    buf = nil.as(String?)
    @@read_requests.send({pointerof(buf), Fiber.current})

    Fiber.suspend

    buf
  end
HertzDevil commented 2 months ago

The current reader loop intentionally does not directly interact with the event loop at all, especially considering it needs to work without -Dpreview_mt.

straight-shoota commented 2 months ago

I figure this shouldn't be an issue anymore with the execution contexts from https://github.com/crystal-lang/rfcs/pull/2 though?

HertzDevil commented 2 months ago

Then we could come back when execution contexts are stabilized. A Windows stable release shall not depend on them.