Smithay / calloop

A callback-based Event Loop
MIT License
176 stars 35 forks source link

Port to rustix from nix #131

Closed notgull closed 1 year ago

notgull commented 1 year ago

Ports parts of this crate from nix to rustix, which uses I/O safety by default and allows for the inlining of syscalls.

WIP because there are some features we need that rustix does not support yet.

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 96.66% and project coverage change: -0.03% :warning:

Comparison is base (d6bba12) 88.81% compared to head (d8f50c7) 88.79%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #131 +/- ## ========================================== - Coverage 88.81% 88.79% -0.03% ========================================== Files 14 14 Lines 1672 1669 -3 ========================================== - Hits 1485 1482 -3 Misses 187 187 ``` | [Flag](https://app.codecov.io/gh/Smithay/calloop/pull/131/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Smithay) | Coverage Δ | | |---|---|---| | [macos-latest](https://app.codecov.io/gh/Smithay/calloop/pull/131/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Smithay) | `88.45% <100.00%> (-0.04%)` | :arrow_down: | | [ubuntu-latest](https://app.codecov.io/gh/Smithay/calloop/pull/131/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Smithay) | `88.37% <95.00%> (-0.01%)` | :arrow_down: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Smithay#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files Changed](https://app.codecov.io/gh/Smithay/calloop/pull/131?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Smithay) | Coverage Δ | | |---|---|---| | [src/error.rs](https://app.codecov.io/gh/Smithay/calloop/pull/131?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Smithay#diff-c3JjL2Vycm9yLnJz) | `0.00% <ø> (ø)` | | | [src/loop\_logic.rs](https://app.codecov.io/gh/Smithay/calloop/pull/131?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Smithay#diff-c3JjL2xvb3BfbG9naWMucnM=) | `90.45% <ø> (ø)` | | | [src/sources/mod.rs](https://app.codecov.io/gh/Smithay/calloop/pull/131?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Smithay#diff-c3JjL3NvdXJjZXMvbW9kLnJz) | `95.69% <ø> (ø)` | | | [src/sources/ping/eventfd.rs](https://app.codecov.io/gh/Smithay/calloop/pull/131?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Smithay#diff-c3JjL3NvdXJjZXMvcGluZy9ldmVudGZkLnJz) | `92.40% <80.00%> (ø)` | | | [src/io.rs](https://app.codecov.io/gh/Smithay/calloop/pull/131?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Smithay#diff-c3JjL2lvLnJz) | `82.55% <100.00%> (-0.21%)` | :arrow_down: | | [src/sources/ping/pipe.rs](https://app.codecov.io/gh/Smithay/calloop/pull/131?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Smithay#diff-c3JjL3NvdXJjZXMvcGluZy9waXBlLnJz) | `98.55% <100.00%> (-0.05%)` | :arrow_down: | | [src/sources/signals.rs](https://app.codecov.io/gh/Smithay/calloop/pull/131?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Smithay#diff-c3JjL3NvdXJjZXMvc2lnbmFscy5ycw==) | `80.45% <100.00%> (+0.22%)` | :arrow_up: |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

notgull commented 1 year ago

Unfortunately it looks like you really do need to talk to libc for signals. So I'll keep nix around for that, but make it optional, as it looks like none of calloop's dependencies explicitly use it.

ids1024 commented 1 year ago

Looking at signals.rs, I guess that's for pthreads stuff?

Looks like thread_block and thread_unblock in nix call pthread_sigmask, and looking at musl, that's a wrapper around the syscall SYS_rt_sigprocmask. So if the few other calls there are similar, it should be possible to do with directly syscalls on Linux (and library calls elsewhere). Other pthreads APIs may not be as thin a wrapper over system calls.

notgull commented 1 year ago

IIRC the "wrapper" in Musl is a lot less thin than you would expect. See this issue in rustix for more information, as well as the undocumented sigprocmask function currently in rustix::runtime. So we probably shouldn't do that.

notgull commented 1 year ago

Looks like CI is passing. @elinorbgr What are your thoughts?