Add PidFd::{kill, wait, try_wait} by the8472 · Pull Request #124101 · rust-lang/rust

the8472 · 2024-04-17T23:59:51Z

#117957 changed Child kill/wait/try_wait to use its pidfd instead of the pid, when one is available.
This PR extracts those implementations and makes them available on PidFd directly.

The PidFd implementations differ significantly from the corresponding Child methods:

the methods can be called after the child has been reaped, which will result in an error but will be safe. This state is not observable in Child unless something stole the zombie child
the ExitStatus is not kept, meaning that only the first time a wait succeeds it will be returned
wait does not close stdin
wait only requires &self instead of &mut self since there is no state to maintain and subsequent calls are safe

Tracking issue: #82971

rustbot · 2024-04-17T23:59:59Z

r? @cuviper

rustbot has assigned @cuviper.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

cuviper · 2024-05-01T23:29:22Z

If you call child.pidfd().wait() (or with take_pidfd()), doesn't that break child.wait()?

the8472 · 2024-05-01T23:37:29Z

Good point. It shouldn't break it immediately since the pid won't be recycled until the fd is dropped (not confident, haven't tested this). But once one does drop it, yeah it'd become an issue.

But people do want to take out pidfds, e.g. for process management in tokio.
So either we have to make it take_pidfd(self) so it consumes the child or try_clone the fd instead so the child can hold onto its own copy.

the8472 · 2024-06-13T22:14:42Z

I've replaced take_pidfd with into_pidfd which consumes the child.

cuviper · 2024-06-20T19:04:18Z

OK, into_pidfd() is better to make sure we avoid pid reuse on the child, but there's still an issue with the saved status. That is, child.pidfd().wait() cannot save the status, so it's lost from child.wait(). (Ditto try_wait())

At the very least, this should be an unresolved question on the tracking issue. Apart from that, r=me.

the8472 · 2024-06-20T19:10:29Z

Yes it'll be lost but in those cases calling Child::wait will return an error as PidFd::wait would, not an incorrect value.
This is indeed new behavior, but any code using PidFd will also be new.

I'll note it in the tracking issue.

@bors r=cuviper rollup

bors · 2024-06-20T19:10:31Z

📌 Commit 210400a has been approved by cuviper

It is now in the queue for this repository.

Add PidFd::{kill, wait, try_wait} rust-lang#117957 changed `Child` kill/wait/try_wait to use its pidfd instead of the pid, when one is available. This PR extracts those implementations and makes them available on `PidFd` directly. The `PidFd` implementations differ significantly from the corresponding `Child` methods: * the methods can be called after the child has been reaped, which will result in an error but will be safe. This state is not observable in `Child` unless something stole the zombie child * the `ExitStatus` is not kept, meaning that only the first time a wait succeeds it will be returned * `wait` does not close stdin * `wait` only requires `&self` instead of `&mut self` since there is no state to maintain and subsequent calls are safe Tracking issue: rust-lang#82971

…kingjubilee Rollup of 9 pull requests Successful merges: - rust-lang#124101 (Add PidFd::{kill, wait, try_wait}) - rust-lang#126125 (Improve conflict marker recovery) - rust-lang#126481 (Add `powerpc-unknown-openbsd` maintaince status) - rust-lang#126613 (Print the tested value in int_log tests) - rust-lang#126617 (Expand `avx512_target_feature` to include VEX variants) - rust-lang#126707 (Pass target to inaccessible-temp-dir rmake test) - rust-lang#126712 (Migrate `relocation-model`, `error-writing-dependencies` and `crate-name-priority` `run-make` tests to rmake) - rust-lang#126757 (Properly gate `safe` keyword in pre-expansion) - rust-lang#126758 (Do not allow safe/unsafe on static and fn items) r? `@ghost` `@rustbot` modify labels: rollup

Add PidFd::{kill, wait, try_wait} rust-lang#117957 changed `Child` kill/wait/try_wait to use its pidfd instead of the pid, when one is available. This PR extracts those implementations and makes them available on `PidFd` directly. The `PidFd` implementations differ significantly from the corresponding `Child` methods: * the methods can be called after the child has been reaped, which will result in an error but will be safe. This state is not observable in `Child` unless something stole the zombie child * the `ExitStatus` is not kept, meaning that only the first time a wait succeeds it will be returned * `wait` does not close stdin * `wait` only requires `&self` instead of `&mut self` since there is no state to maintain and subsequent calls are safe Tracking issue: rust-lang#82971

…iaskrgr Rollup of 8 pull requests Successful merges: - rust-lang#124101 (Add PidFd::{kill, wait, try_wait}) - rust-lang#126125 (Improve conflict marker recovery) - rust-lang#126481 (Add `powerpc-unknown-openbsd` maintenance status) - rust-lang#126613 (Print the tested value in int_log tests) - rust-lang#126617 (Expand `avx512_target_feature` to include VEX variants) - rust-lang#126700 (Make edition dependent `:expr` macro fragment act like the edition-dependent `:pat` fragment does) - rust-lang#126707 (Pass target to inaccessible-temp-dir rmake test) - rust-lang#126767 (`StaticForeignItem` and `StaticItem` are the same) r? `@ghost` `@rustbot` modify labels: rollup

matthiaskrgr · 2024-06-21T07:12:06Z

@bors r- #126775 (comment)

workingjubilee · 2024-06-21T08:06:07Z

@bors rollup=iffy

As long as a pidfd is on a child it can be safely reaped. Taking it would mean the child would now have to be awaited through its pid, but could also be awaited through the pidfd. This could then suffer from a recycling race.

the8472 · 2024-06-21T22:59:53Z

fixed the import, docs build for x86_64-pc-windows-gnu

@bors r=cuviper

bors · 2024-06-21T22:59:55Z

📌 Commit 8abf149 has been approved by cuviper

It is now in the queue for this repository.

bors · 2024-06-21T23:51:22Z

⌛ Testing commit 8abf149 with merge 06c6b72...

Add PidFd::{kill, wait, try_wait} rust-lang#117957 changed `Child` kill/wait/try_wait to use its pidfd instead of the pid, when one is available. This PR extracts those implementations and makes them available on `PidFd` directly. The `PidFd` implementations differ significantly from the corresponding `Child` methods: * the methods can be called after the child has been reaped, which will result in an error but will be safe. This state is not observable in `Child` unless something stole the zombie child * the `ExitStatus` is not kept, meaning that only the first time a wait succeeds it will be returned * `wait` does not close stdin * `wait` only requires `&self` instead of `&mut self` since there is no state to maintain and subsequent calls are safe Tracking issue: rust-lang#82971

rust-log-analyzer · 2024-06-22T00:38:33Z

The job x86_64-msvc failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

failures:

---- [ui] tests\ui\lto\lto-duplicate-symbols.rs stdout ----

error: auxiliary build of "C:\\a\\rust\\rust\\tests\\ui\\lto\\auxiliary\\lto-duplicate-symbols1.rs" failed to compile: 
status: exit code: 1
command: PATH="C:\a\rust\rust\build\x86_64-pc-windows-msvc\stage2\bin;C:\Program Files (x86)\Windows Kits\10\bin\x64;C:\Program Files (x86)\Windows Kits\10\bin\10.0.22621.0\x64;C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.40.33807\bin\HostX64\x64;C:\Program Files\Microsoft Visual Studio\2022\Enterprise\VC\Tools\MSVC\14.40.33807\bin\HostX64\x64;C:\a\rust\rust\build\x86_64-pc-windows-msvc\stage0-bootstrap-tools\x86_64-pc-windows-msvc\release\deps;C:\a\rust\rust\build\x86_64-pc-windows-msvc\stage0\bin;C:\Program Files\PowerShell\7;C:\a\_temp\msys64\mingw64\bin;C:\a\_temp\msys64\usr\local\bin;C:\a\_temp\msys64\usr\bin;C:\a\_temp\msys64\usr\bin;C:\a\rust\rust\ninja;C:\a\rust\rust\sccache;C:\a\_temp\setup-msys2;C:\Program Files\MongoDB\Server\5.0\bin;C:\aliyun-cli;C:\vcpkg;C:\Program Files (x86)\NSIS;C:\tools\zstd;C:\Program Files\Mercurial;C:\hostedtoolcache\windows\stack\2.15.7\x64;C:\cabal\bin;C:\ghcup\bin;C:\mingw64\bin;C:\Program Files\dotnet;C:\Program Files\MySQL\MySQL Server 8.0\bin;C:\Program Files\R\R-4.4.0\bin\x64;C:\SeleniumWebDrivers\GeckoDriver;C:\SeleniumWebDrivers\EdgeDriver;C:\SeleniumWebDrivers\ChromeDriver;C:\Program Files (x86)\sbt\bin;C:\Program Files (x86)\GitHub CLI;C:\Program Files\Git\bin;C:\Program Files (x86)\pipx_bin;C:\npm\prefix;C:\hostedtoolcache\windows\go\1.21.11\x64\bin;C:\hostedtoolcache\windows\Python\3.9.13\x64\Scripts;C:\hostedtoolcache\windows\Python\3.9.13\x64;C:\hostedtoolcache\windows\Ruby\3.0.7\x64\bin;C:\Program Files\OpenSSL\bin;C:\tools\kotlinc\bin;C:\hostedtoolcache\windows\Java_Temurin-Hotspot_jdk\8.0.412-8\x64\bin;C:\Program Files\ImageMagick-7.1.1-Q16-HDRI;C:\Program Files\Microsoft SDKs\Azure\CLI2\wbin;C:\ProgramData\kind;C:\ProgramData\docker-compose;C:\ProgramData\Chocolatey\bin;C:\Windows\system32;C:\Windows;C:\Windows\System32\Wbem;C:\Windows\System32\WindowsPowerShell\v1.0;C:\Windows\System32\OpenSSH;C:\Program Files\dotnet;C:\Program Files\PowerShell\7;C:\Program Files\Microsoft\Web Platform Installer;C:\Program Files\TortoiseSVN\bin;C:\Program Files\Microsoft SQL Server\Client SDK\ODBC\170\Tools\Binn;C:\Program Files\Microsoft SQL Server\150\Tools\Binn;C:\Program Files (x86)\Windows Kits\10\Windows Performance Toolkit;C:\Program Files (x86)\WiX Toolset v3.14\bin;C:\Program Files\Microsoft SQL Server\130\DTS\Binn;C:\Program Files\Microsoft SQL Server\140\DTS\Binn;C:\Program Files\Microsoft SQL Server\150\DTS\Binn;C:\Program Files\Microsoft SQL Server\160\DTS\Binn;C:\Strawberry\c\bin;C:\Strawberry\perl\site\bin;C:\Strawberry\perl\bin;C:\ProgramData\chocolatey\lib\pulumi\tools\Pulumi\bin;C:\Program Files\CMake\bin;C:\ProgramData\chocolatey\lib\maven\apache-maven-3.8.7\bin;C:\Program Files\Microsoft Service Fabric\bin\Fabric\Fabric.Code;C:\Program Files\Microsoft SDKs\Service Fabric\Tools\ServiceFabricLocalClusterManager;C:\Program Files\nodejs;C:\Program Files\Git\cmd;C:\Program Files\Git\mingw64\bin;C:\Program Files\Git\usr\bin;C:\Program Files\GitHub CLI;C:\tools\php;C:\Program Files (x86)\sbt\bin;C:\Program Files\Amazon\AWSCLIV2;C:\Program Files\Amazon\SessionManagerPlugin\bin;C:\Program Files\Amazon\AWSSAMCLI\bin;C:\Program Files\Microsoft SQL Server\130\Tools\Binn;C:\Program Files\LLVM\bin;C:\Users\runneradmin\.dotnet\tools;C:\Users\runneradmin\.cargo\bin;C:\Users\runneradmin\AppData\Local\Microsoft\WindowsApps;C:\a\_temp\msys64\usr\bin\site_perl;C:\a\_temp\msys64\usr\bin\vendor_perl;C:\a\_temp\msys64\usr\bin\core_perl" "C:\\a\\rust\\rust\\build\\x86_64-pc-windows-msvc\\stage2\\bin\\rustc.exe" "C:\\a\\rust\\rust\\tests\\ui\\lto\\auxiliary\\lto-duplicate-symbols1.rs" "-Zthreads=1" "-Zsimulate-remapped-rust-src-base=/rustc/FAKE_PREFIX" "-Ztranslate-remapped-path-to-local-path=no" "-Z" "ignore-directory-in-diagnostics-source-blocks=C:\\Users\\runneradmin\\.cargo" "-Z" "ignore-directory-in-diagnostics-source-blocks=C:\\a\\rust\\rust\\vendor" "--sysroot" "C:\\a\\rust\\rust\\build\\x86_64-pc-windows-msvc\\stage2" "--target=x86_64-pc-windows-msvc" "--check-cfg" "cfg(FALSE)" "--error-format" "json" "--json" "future-incompat" "-Ccodegen-units=1" "-Zui-testing" "-Zdeduplicate-diagnostics=no" "-Zwrite-long-types-to-disk=no" "-Cstrip=debuginfo" "--out-dir" "C:\\a\\rust\\rust\\build\\x86_64-pc-windows-msvc\\test\\ui\\lto\\lto-duplicate-symbols\\auxiliary" "-A" "internal_features" "-Crpath" "-Cdebuginfo=0" "-Lnative=C:\\a\\rust\\rust\\build\\x86_64-pc-windows-msvc\\native\\rust-test-helpers" "-L" "C:\\a\\rust\\rust\\build\\x86_64-pc-windows-msvc\\test\\ui\\lto\\lto-duplicate-symbols\\auxiliary"
--- stderr -------------------------------
--- stderr -------------------------------
error: couldn't create a temp dir: Access is denied. (os error 5) at path "C:\\a\\_temp\\msys64\\tmp\\rustcMwb8Le"
error: aborting due to 1 previous error
------------------------------------------


---
test result: FAILED. 16875 passed; 1 failed; 250 ignored; 0 measured; 2 filtered out; finished in 379.97s

Some tests failed in compiletest suite=ui mode=ui host=x86_64-pc-windows-msvc target=x86_64-pc-windows-msvc
Build completed unsuccessfully in 0:40:48
make: *** [Makefile:102: ci-msvc-ps1] Error 1
  network time: Sat, 22 Jun 2024 00:38:20 GMT
##[error]Process completed with exit code 2.
Post job cleanup.
[command]"C:\Program Files\Git\bin\git.exe" version

bors · 2024-06-22T00:39:07Z

💔 Test failed - checks-actions

cuviper · 2024-06-22T01:40:22Z

@bors retry

bors · 2024-06-22T03:35:55Z

⌛ Testing commit 8abf149 with merge 10e1f5d...

bors · 2024-06-22T05:50:49Z

☀️ Test successful - checks-actions
Approved by: cuviper
Pushing 10e1f5d to master...

rustbot assigned cuviper Apr 17, 2024

rustbot added O-linux Operating system: Linux O-unix Operating system: Unix-like S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Apr 18, 2024

the8472 force-pushed the pidfd-methods branch from 43c6d01 to cbd0dab Compare April 18, 2024 00:06

cuviper reviewed May 1, 2024

View reviewed changes

Comment thread library/std/src/os/linux/process.rs Outdated

the8472 force-pushed the pidfd-methods branch from cbd0dab to 02d9995 Compare June 13, 2024 22:03

the8472 requested a review from cuviper June 13, 2024 22:14

the8472 force-pushed the pidfd-methods branch from 02d9995 to b5be21d Compare June 13, 2024 22:15

This comment has been minimized.

Sign in to view

traviscross added the F-linux_pidfd `#![feature(linux_pidfd)]` label Jun 13, 2024

the8472 force-pushed the pidfd-methods branch from b5be21d to 210400a Compare June 15, 2024 08:32

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 Jun 20, 2024

the8472 mentioned this pull request Jun 20, 2024

Tracking Issue for linux_pidfd #82971

Open

10 tasks

workingjubilee mentioned this pull request Jun 21, 2024

Rollup of 9 pull requests #126771

Closed

matthiaskrgr mentioned this pull request Jun 21, 2024

Rollup of 8 pull requests #126775

Closed

bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jun 21, 2024

the8472 added 2 commits June 22, 2024 00:46

Add PidFd::{kill, wait, try_wait}

0787c73

to extract a pidfd we must consume the child

8abf149

As long as a pidfd is on a child it can be safely reaped. Taking it would mean the child would now have to be awaited through its pid, but could also be awaited through the pidfd. This could then suffer from a recycling race.

the8472 force-pushed the pidfd-methods branch from 210400a to 8abf149 Compare June 21, 2024 22:47

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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 21, 2024

bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Jun 22, 2024

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 Jun 22, 2024

bors added the merged-by-bors This PR was explicitly merged by bors. label Jun 22, 2024

bors merged commit 10e1f5d into rust-lang:master Jun 22, 2024

rustbot ad

Uh oh!

Conversation

the8472 commented Apr 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rustbot commented Apr 17, 2024

Uh oh!

Uh oh!

cuviper commented May 1, 2024

Uh oh!

the8472 commented May 1, 2024

Uh oh!

the8472 commented Jun 13, 2024

Uh oh!

This comment has been minimized.

cuviper commented Jun 20, 2024

Uh oh!

the8472 commented Jun 20, 2024

Uh oh!

bors commented Jun 20, 2024

Uh oh!

matthiaskrgr commented Jun 21, 2024

Uh oh!

workingjubilee commented Jun 21, 2024

Uh oh!

the8472 commented Jun 21, 2024

Uh oh!

bors commented Jun 21, 2024

Uh oh!

bors commented Jun 21, 2024

Uh oh!

rust-log-analyzer commented Jun 22, 2024

Uh oh!

bors commented Jun 22, 2024

Uh oh!

cuviper commented Jun 22, 2024

Uh oh!

bors commented Jun 22, 2024

Uh oh!

bors commented Jun 22, 2024

Uh oh!

the8472 commented Apr 17, 2024 •

edited

Loading