miri icon indicating copy to clipboard operation
miri copied to clipboard

Complete and fix our eventfd implementation

Open RalfJung opened this issue 1 year ago • 6 comments

eventfd is documented reasonably well in its man page. The current implementation in Miri is incomplete, doesn't have tests (it is only indirect exercised via the tokio MVP test but that's not a good way to test a specific low-level API), and is partially wrong.

It is incomplete in that read is not implemented, which means they can't actually be used to do anything. That's also where EFD_NONBLOCK would come in.

It is partially wrong in that

  • write panics in all sorts of situations that should be handled gracefully
  • dup creates a new independent event object, rather than a second FD that refers to the same event object

Work on this should IMO follow the proposed "project" process.

RalfJung avatar Apr 03 '24 13:04 RalfJung

Also the check here is bogus:

https://github.com/rust-lang/miri/blob/99be2b7c7c4af7dfbfc7447289d1637e633f09e6/src/shims/unix/linux/eventfd.rs#L109-L111

That should be flags & !(efd_cloexec | efd_nonblock | efd_semaphore) != 0.

EDIT: but turns out we have the flags tokio needs.

RalfJung avatar Apr 03 '24 13:04 RalfJung

dup creates a new independent event object, rather than a second FD that refers to the same event object

This should probably be fixed by implementing a proper notion of "file description", and making the file descriptor table a simple indirection for file descriptions. That would fix dup for all FDs once and for all, and it will anyway be needed for epoll.

RalfJung avatar Apr 04 '24 06:04 RalfJung

Hey @RalfJung I would like to give this a shot. May I just @rustbot claim it?

PartiallyUntyped avatar Apr 05 '24 07:04 PartiallyUntyped

Please coordinate with @oli-obk -- I saw something about a potential GSoC project for epoll support, and I assume this would be part of it.

RalfJung avatar Apr 05 '24 08:04 RalfJung

cc @tiif

There's probably some parallel development that can be done, but the file descriptor table refactor is likely gonna block other work, so the earlier it is done the better.

oli-obk avatar Apr 05 '24 09:04 oli-obk

I think it's probably for the best if I pick something simpler and isolated to start with. Thank you!

PartiallyUntyped avatar Apr 05 '24 09:04 PartiallyUntyped

@rustbot claim

tiif avatar May 05 '24 09:05 tiif

This hackmd contains the details for eventfd implementation: https://hackmd.io/@tiif/rk9hlmP4R

tiif avatar Jun 02 '24 12:06 tiif