socketry / async

An awesome asynchronous event-driven reactor for Ruby.
MIT License
2.12k stars 86 forks source link

Using File.read with async in Windows causes `Bad file descriptor` Exception #206

Open femshima opened 1 year ago

femshima commented 1 year ago

Description

When I use File.read in Async, the following exception is thrown.

require "async"

Async do |task|
  task.async do
    p File.read("README.md")
  end
end
  0.0s     warn: Async::Task [oid=0x348] [ec=0x35c] [pid=4880] [2022-12-06 13:02:07 +0900]
               | Task may have ended with unhandled exception.
               |   Errno::EBADF: Bad file descriptor
               |   → <internal:io> 63
               |     C:/Ruby31-x64/lib/ruby/gems/3.1.0/gems/io-event-1.1.3/lib/io/event/selector/select.rb 206

This behavior is only observed in Windows with ruby 3.1.

Also, I reproduced this behavior in GitHub Actions here.

Sample Code

require "async"

Async do |task|
  task.async do
    p File.read("README.md")
  end
end

Expected Behavior

No exception is thrown, and the content of README.md is shown on console.

Actual Behavior

Errno::EBADF: Bad file descriptor exception is thrown, and the task stops there.

Versions

Windows 10 Home 21H2 ruby 3.1.1p18 (2022-02-18 revision 53f5fc4236) [x64-mingw-ucrt] async (2.3.0)

ioquatix commented 1 year ago

I guess it's a limitation of Windows. The best thing we can do is try to avoid doing any kind of asynchronous file IO, which isn't supported anyway in this situation. The simple solution is to just detect windows and skip any kind of asynchronous io_read and io_write operations.

femshima commented 1 year ago

Then I'll just try detecting Windows and avoid async file IO on it...

Thank you for your comment and advice. I'm closing this issue.

ioquatix commented 1 year ago

Async on Windows is a work in progress. The first part was to get io-event working which is tested, but async itself is not well tested on Windows yet. We should probably disable the io_read and io_write hooks on Windows for this reason. I want to leave this issue open until we do that.

ioquatix commented 1 year ago

https://github.com/socketry/async/pull/184 now includes the above.