Skip to content

[beta] Fix TcpListener::accept() on x86 Android on beta by disabling the use of accept4. #82476

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

Merged
merged 1 commit into from
Mar 5, 2021

Conversation

de-vri-es
Copy link
Contributor

This is the same as #82475, but for beta.

In a nutshell: TcpListener::accept is broken on Android x86 on stable and beta because it performs a raw accept4 syscall, which doesn't exist on that platform. This was originally reported in #82400, so you can find more details there.

@rustbot label +O-android
r? @Mark-Simulacrum

On x86 before Linux 4.3, accept4 is not a separate syscall.
Instead, it can be called using `socketcall(SYS_ACCEPT4, ...).
Rather than implementing that here, just fall back to `accept`.
@rustbot rustbot added the O-android Operating system: Android label Feb 24, 2021
@rust-highfive
Copy link
Contributor

⚠️ Warning ⚠️

  • Pull requests are usually filed against the master branch for this repo, but this one is against beta. Please double check that you specified the right target!

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 24, 2021
@Mark-Simulacrum Mark-Simulacrum changed the title Fix TcpListener::accept() on x86 Android on beta by disabling the use of accept4. [beta] Fix TcpListener::accept() on x86 Android on beta by disabling the use of accept4. Mar 1, 2021
@Mark-Simulacrum Mark-Simulacrum added beta-nominated Nominated for backporting to the compiler in the beta channel. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Mar 1, 2021
@Mark-Simulacrum
Copy link
Member

Similarly to the stable PR, nominated for beta backport (really just merging, as the PR already targets beta branch), but also reassigning to @joshtriplett who reviewed parts of this in the libc repository. r? @joshtriplett

@joshtriplett
Copy link
Member

As with the stable PR, this seems like a reasonable approach to minimize changes. I prefer the approach of switching to socketcall, but that's more invasive, so this seems fine for beta.

@bors r+

@bors
Copy link
Collaborator

bors commented Mar 1, 2021

📌 Commit 695b048 has been approved by joshtriplett

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 1, 2021
@apiraino apiraino added the beta-accepted Accepted for backporting to the compiler in the beta channel. label Mar 4, 2021
@m-ou-se
Copy link
Member

m-ou-se commented Mar 5, 2021

@bors rollup=never

@Dylan-DPC-zz
Copy link

@bors p=1

@bors
Copy link
Collaborator

bors commented Mar 5, 2021

⌛ Testing commit 695b048 with merge 4d25f46...

@bors
Copy link
Collaborator

bors commented Mar 5, 2021

☀️ Test successful - checks-actions
Approved by: joshtriplett
Pushing 4d25f46 to beta...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Mar 5, 2021
@bors bors merged commit 4d25f46 into rust-lang:beta Mar 5, 2021
@rustbot rustbot added this to the 1.51.0 milestone Mar 5, 2021
@cuviper cuviper removed the beta-nominated Nominated for backporting to the compiler in the beta channel. label Mar 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
beta-accepted Accepted for backporting to the compiler in the beta channel. merged-by-bors This PR was explicitly merged by bors. O-android Operating system: Android S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants