ocaml-multicore / eio

Effects-based direct-style IO for multicore OCaml
Other
550 stars 66 forks source link

Add Eio_unix.Net module #516

Closed talex5 closed 1 year ago

talex5 commented 1 year ago

I was planning to include FD passing in this FD, but it got too long so I've split that out for later.

Fixes #342.

talex5 commented 1 year ago

Having tried this out for a bit, this API is a little annoying because we also need unix equivalents to Net.run_server, etc. So I think it might be better to make the main Eio types polymorphic instead.

talex5 commented 1 year ago

I've removed the FD passing bits for now and put them at https://github.com/talex5/eio/tree/fd-passing, which I'll submit separately once this is merged. So this PR is just adding a skeleton Eio_unix.Net and sorting out the stream/datagram confusion now, which should make it easier to review.

talex5 commented 1 year ago

I've pushed a couple of minor changes:

If there are no objections I'll merge this tomorrow, so I can then submit the FD-passing PR.

avsm commented 1 year ago

One implication of this change is that Eio.Net.accept_fork and other connection handlers now must implement close (whereas previously it was just a Flow.two_way).

avsm commented 1 year ago

(For reference, this is the diff I applied to bring other libraries uptodate with this change https://github.com/avsm/eeww/commit/dfdd8f248b249254b63bc5f18f8ab172ec94ca0e)

talex5 commented 1 year ago

Good point. At the moment, accept_fork always closes the flow when the handler finishes, so if you close it manually then that will fail, which is why we didn't have close here before. Perhaps we should revert the addition of close here.

avsm commented 1 year ago

It would be cleaner to just pass a Flow.two_way I think, and leave closing to the connection handler.