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.

Sign up for GitHub

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

Jump to bottom

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

Closed
lambdageek opened this issue Jan 28, 2021 · 3 comments
Closed

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

lambdageek opened this issue Jan 28, 2021 · 3 comments
Labels
area-Runtime: PAL

Comments

@lambdageek
Copy link
Member

lambdageek commented Jan 28, 2021 •
edited
Loading

(Thanks to @Therzok for filing a related issue to dotnet/runtime: dotnet/runtime#47584 )

Note the issue is different in mono/mono due to Thread.Abort and Thread.Interrupt

Description

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

The 3 syscalls are the only ones in pal_io.c that are not handling EINTR in native, but they also don't handle it in managed.

https://github.com/mono/corefx/blob/7c24bb0756fd39fbf09b8777f25c15d21d78eb46/src/Native/Unix/System.Native/pal_io.c#L468

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.
@lambdageek
Copy link
Member Author

lambdageek commented Jan 28, 2021 •
edited
Loading

For mono/mono, due to thread aborts and interrupts, this has ot be handled in a similar way to #7255

We can't just look in native checking for EINTR. The loop has to be moved up to the C# layer so that the thread can be interrupted.

https://github.com/mono/mono/pull/7255/files#diff-72fb55c77960d94ac59663ea63ee66232ec537ef19379b2df9063dfc40d47db7R35-R49

There's a related change to the pal code too

https://github.com/mono/corefx/blob/7c24bb0756fd39fbf09b8777f25c15d21d78eb46/src/Native/Unix/System.Native/pal_io.c#L1134-L1136

Basically the issue is the way mono aborts threads. A thread abort interrupts the thread, breaking syscalls, and sets some state that the thread will examine when it returns to managed. If the thread never returns to managed and just loops in native code, it will never abort. (See mono_thread_internal_abort - async_abort_internal signals the thread and installs the interrupt token which will be examined in managed)

@Therzok
Copy link
Contributor

Therzok commented Mar 1, 2021

@lambdageek dotnet/runtime PR has been merged.

lambdageek added a commit to lambdageek/mono-corefx that referenced this issue Apr 16, 2021
@lambdageek
[System.Native] Exclude ReadDir pinvoke wrappers on Mono; handle EINTR
8e49997 
For Mono we must loop in managed when opendir/readdir/closedir return EINTR in
order to handle Thread.Abort.

This is because while a thread abort will break syscalls with EINTR, throwing the
ThreadAbortException will only happen when the pinvoke returns.

(Although opendir/readdir/clsoedir are not documented to return EINTR, they do
so on macOS Big Sur under certain conditions, so we will need to add managed looping)

Related to mono/mono#20799
This was referenced Apr 16, 2021
Merged
Merged
lambdageek added a commit to lambdageek/mono-corefx that referenced this issue Apr 16, 2021
@lambdageek
[System.Native] Exclude ReadDir pinvoke wrappers on Mono; handle EINTR
e86f4b3 
For Mono we must loop in managed when opendir/readdir/closedir return EINTR in
order to handle Thread.Abort.

This is because while a thread abort will break syscalls with EINTR, throwing the
ThreadAbortException will only happen when the pinvoke returns.

(Although opendir/readdir/clsoedir are not documented to return EINTR, they do
so on macOS Big Sur under certain conditions, so we will need to add managed looping)

Related to mono/mono#20799
lambdageek added a commit to lambdageek/mono that referenced this issue Apr 23, 2021
@lambdageek
[System.Native] Handle ReadDir EINTR
0c1caf7 
Handle EINTR for opendir/readdir/closedir.

Because of how thread aborts work, we must loop in managed.
A thread abort will break syscalls and return EINTR, but mono will not raise a
ThreadAbortException until a pinvoke returns.

(Despite the documentation, opendir/readdir/closedir sometimes return EINTR on
macOS Big Sur.)

Related to mono#20799
lambdageek added a commit to lambdageek/mono that referenced this issue Apr 23, 2021
@lambdageek
[System.Native] Handle ReadDir EINTR
5722b58 
Handle EINTR for opendir/readdir/closedir.

Because of how thread aborts work, we must loop in managed.
A thread abort will break syscalls and return EINTR, but mono will not raise a
ThreadAbortException until a pinvoke returns.

(Despite the documentation, opendir/readdir/closedir sometimes return EINTR on
macOS Big Sur.)

Related to mono#20799
@lambdageek lambdageek mentioned this issue Apr 23, 2021
Merged
lambdageek added a commit that referenced this issue Apr 29, 2021
@lambdageek @akoeplinger
[System.Native] Handle ReadDir EINTR (#21016)
4545a50 
* [System.Native] Handle ReadDir EINTR

Handle EINTR for opendir/readdir/closedir.

Because of how thread aborts work, we must loop in managed.
A thread abort will break syscalls and return EINTR, but mono will not raise a
ThreadAbortException until a pinvoke returns.

(Despite the documentation, opendir/readdir/closedir sometimes return EINTR on
macOS Big Sur.)

Related to #20799

* bump corefx

* [System] on unix, include Interop.ReadDir.Mono.cs

on mac it's in corlib, on unix and android it's in System, for some reason

* Add newline

Co-authored-by: Alexander Köplinger <alex.koeplinger@outlook.com>
lambdageek added a commit that referenced this issue Apr 29, 2021
@lambdageek @akoeplinger
[2020-02][System.Native] Handle ReadDir EINTR (#21029)
51d876a 
* [System.Native] Handle ReadDir EINTR

Handle EINTR for opendir/readdir/closedir.

Because of how thread aborts work, we must loop in managed.
A thread abort will break syscalls and return EINTR, but mono will not raise a
ThreadAbortException until a pinvoke returns.

(Despite the documentation, opendir/readdir/closedir sometimes return EINTR on
macOS Big Sur.)

Related to #20799

* bump corefx

* [System] on unix, include Interop.ReadDir.Mono.cs

on mac it's in corlib, on unix and android it's in System, for some reason

* Add newline

Co-authored-by: Alexander Köplinger <alex.koeplinger@outlook.com>
@lambdageek
Copy link
Member Author

Fixed on mono main and 2020-02

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area-Runtime: PAL
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@Therzok @lambdageek