Work around EPERM fstat error for inotify under WSL#6535
Merged
headius merged 4 commits intojruby:masterfrom Feb 10, 2021
Merged
Work around EPERM fstat error for inotify under WSL#6535headius merged 4 commits intojruby:masterfrom
headius merged 4 commits intojruby:masterfrom
Conversation
The file descriptor created by inotify fails to fstat when running inside the Windows Subsystem for Linux, resulting in an EPERM and breaking JRuby on Rails apps that are using rb-inotify. This patch mimics the CRuby code in preferring fcntl over fstat. Since we cannot easily detect the availability of fcntl, this logic just falls back to fstat for the one platform where we know we can't use fcntl: Windows. Fixes jruby#6129.
headius
force-pushed
the
io_new_use_fcntl
branch
from
4230198 to
1bf41d8
Compare
Member
Author
|
Bit of an issue with this fix in that it swallows errors when attempting to |
Member
Author
|
It might be sufficient to reduce the exception-swallowing catch to just the EPERM we see in WSL, since that would allow EBADF and friends through for bad descriptors and EPERM is not an expected error from fstat. |
EPERM has only been osbserved here when calling fstat against an inotify file descriptor under Windows Subsystem for Linux. Because this is not otherwise expected as an error from fstat, we assume it means this is not a normal file.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The
fstatsystem call raises EPERM when run against an inotify file descriptor under Windows Subsystem for Linux, as reported in #6129.Rails uses inotify support via rb-inotify, which recently started using
IO.newto wrap the inotify file descriptor. Our logic forIO.newtries to usefstatin two places, which causes rb-inotify to fail.There are updates to WSL that purport to improve inotify support, but I have been unable to find a rig that works properly.
This PR makes the following changes in an attempt to deal with the issue:
fcntlexcept on Windows (which only hasfstat). This means we usefcntlunder WSL, avoiding the EPERM.fstatto determine the file type, which we do to skip using selection logic against normal files. The logic I added assumes if we can'tfstatit is probably not a normal file.The second fix is mildly hacky but our fstat there was already hacky.
This allows the reduced script from #6129 to run properly and may fix #6129 sufficiently well.