Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing EINTR handling on macOS for opendir/readdir/closedir syscalls #47584

Closed
Therzok opened this issue Jan 28, 2021 · 7 comments · Fixed by #48694
Closed

Missing EINTR handling on macOS for opendir/readdir/closedir syscalls #47584

Therzok opened this issue Jan 28, 2021 · 7 comments · Fixed by #48694

Comments

@Therzok
Copy link
Contributor

Therzok commented Jan 28, 2021

Description

It seems that under certain conditions, the opendir, readdir, closedir can set errno to EINTR.

These 3 syscalls are the only ones I can see not handling EINTR:

DIR* SystemNative_OpenDir(const char* path)

Note: it is not documented in the man pages, but it seems to be a problem in practice.

Configuration

Mono 2020-02, macOS, x64. Not specific to this.

Regression?

Only started happening with macOS Big Sur.

Other information

See this gist for an exception being bubbled on mono which is using the same source as dotnet/runtime.

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@danmoseley
Copy link
Member

closedir does document EINTR at least here https://www.man7.org/linux/man-pages/man3/closedir.3p.html. Not sure about the others.

@Therzok
Copy link
Contributor Author

Therzok commented Jan 29, 2021

opendir is the one being called in the exception stack trace linked above, so I think it's fair to consider it set.

@danmoseley
Copy link
Member

@janvorli any thoghts here? I guess worst case, it is unnecessary code. My only thought (waves hands) that if EINTR is actually a bug here, then who knows if it would just continue to return EINTR, and we would hang.

@janvorli
Copy link
Member

It seems that opendir can return EINTR on macOS as far as I can tell from https://opensource.apple.com/source/Libc/Libc-1439.40.11/gen/FreeBSD/opendir.c.auto.html. It internally calls fstatfs which can return EINTR according to its documentation.
As for closedir, it calls regular close on a handle, thus the behavior of close controls the error code. There is an interesting discussion on EINTR and close and it seems that it should be ignored. See https://sourceware.org/bugzilla/show_bug.cgi?id=14627.
Regarding the readdir, it is not clear to me whether it can return EINTR or not.

Overall, it seems to me it would be safe to handle it at opendir / readdir and eat it at closedir.

@danmoseley
Copy link
Member

Thanks @janvorli . @Therzok would you like to offer a PR?

@Therzok
Copy link
Contributor Author

Therzok commented Jan 31, 2021

@janvorli I dug a bit into the macos source code and the , and via: getdirentries64 -> getdirentriescommon -> vnode_getwithref -> vnode_getiocount -> msleep -> _sleep we got a possible EINTR.

I'll send a PR with the fixes.

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Feb 24, 2021
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Feb 25, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Mar 27, 2021
@marek-safar marek-safar removed the untriaged New issue has not been triaged by the area owner label Apr 16, 2021
@marek-safar marek-safar added this to the 6.0.0 milestone Apr 16, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants