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

Don't tolerate EBADF on close. #2002

Merged
merged 1 commit into from
Dec 3, 2021
Merged

Don't tolerate EBADF on close. #2002

merged 1 commit into from
Dec 3, 2021

Conversation

Lukasa
Copy link
Contributor

@Lukasa Lukasa commented Dec 3, 2021

Motivation:

It's like that #1999 was a little wide, and we should have forbidden
EBDAF on close.

Modifications:

Still forbid EBADF on close.

Result:

EBADF still preconditions on close.
Fixes #2001.

Motivation:

It's like that apple#1999 was a little wide, and we should have forbidden
EBDAF on close.

Modifications:

Still forbid EBADF on close.

Result:

EBADF still preconditions on close.
@Lukasa Lukasa added the patch-version-bump-only For PRs that when merged will only cause a bump of the patch version, ie. 1.0.x -> 1.0.(x+1) label Dec 3, 2021
@Lukasa Lukasa requested a review from weissi December 3, 2021 08:54
@weissi
Copy link
Member

weissi commented Dec 3, 2021

Thank you @Lukasa

@weissi
Copy link
Member

weissi commented Dec 3, 2021

@Lukasa/@John-Connolly turns out there is a SO_ISDEFUNCT getsockopt which we use to query if the socket is actually EBADF or just defunct. So we could change the code on iOS to something to check SO_ISDEFUNCT on iOS if we receive an EBADF.

@Lukasa
Copy link
Contributor Author

Lukasa commented Dec 3, 2021

I added a comment to the check that mention this. However, I don't think it's that simple: in the case of accept we'd have to check EBADF on the listening socket, not the one that hit the error. Actually doing this makes the code quite hard, I think, and we get most of the improvement from this version of the patch, so I consider it a future enhancement.

@weissi
Copy link
Member

weissi commented Dec 3, 2021

@Lukasa works for me!

Copy link
Member

@weissi weissi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you!

@Lukasa Lukasa merged commit d3a6510 into apple:main Dec 3, 2021
@Lukasa Lukasa deleted the cb-modify-ebadf branch December 3, 2021 09:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
patch-version-bump-only For PRs that when merged will only cause a bump of the patch version, ie. 1.0.x -> 1.0.(x+1)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

EBADF relaxation too much
2 participants