Skip to content
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

Account for stack differences in the socketpair test (issue #312) #313

Merged
merged 1 commit into from
Sep 20, 2022

Conversation

haesbaert
Copy link
Contributor

Linux returns ECONNRESET while every other stack tested returns EOF on this test.

I did not dig into the especification but Linux appears to be on the weird side.

In order to get a ECONNRESET, the write needs to hit the socket after the close, but our program does:

WRITE(A)
CLOSE(B)
READ(A)

Linux never gives "A" side a chance to see a proper EOF, while the others do:

Linux -> ECONNRESET
FreeBSD 12 -> EOF
OpenBSD 7.2 -> EOF
macos 12.5.1 -> EOF

We can fix this by properly draining B side of things before the close, but we can't reliably test ECONNRESET so turn this into an EOF test for now.

@talex5
Copy link
Collaborator

talex5 commented Sep 9, 2022

The test is specifically trying to test for this error, though (i.e. where data may have been lost). I found some more info at https://stackoverflow.com/questions/2974021/what-does-econnreset-mean-in-the-context-of-an-af-local-socket

I suggest performing the same operations as before, but adding | End_of_file to allow that outcome too for non-Linux systems.

…ticore#312)

Linux returns ECONNRESET while every other stack tested returns EOF on this
test.

I did not dig into the especification but Linux appears to be on the weird side.

In order to get a ECONNRESET, the write needs to hit the socket _after_ the
close, but our program does:

WRITE(A)
CLOSE(B)
READ(A)

Linux never gives "A" side a chance to see a proper EOF, while the others do:

Linux        -> ECONNRESET
FreeBSD 12   -> EOF
OpenBSD 7.2  -> EOF
macos 12.5.1 -> EOF

Until we can reliably guarantee the same return on all stacks, relax the test a
bit and accept EOF.
Copy link
Collaborator

@talex5 talex5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@talex5 talex5 merged commit bdf1753 into ocaml-multicore:main Sep 20, 2022
talex5 added a commit to talex5/opam-repository that referenced this pull request Oct 12, 2022
CHANGES:

Changes:

- Update to OCaml 5.0.0~beta1 (@anmonteiro @talex5 ocaml-multicore/eio#346).

- Add API for seekable read/writes (@nojb ocaml-multicore/eio#307).

- Add `Flow.write` (@haesbaert ocaml-multicore/eio#318).
  This provides an optimised alternative to `copy` in the case where you are writing from a buffer.

- Add `Net.with_tcp_connect` (@bikallem ocaml-multicore/eio#302).
  Convenience function for opening a TCP connection.

- Add `Eio.Time.Timeout` (@talex5 ocaml-multicore/eio#320).
  Makes it easier to pass timeouts around.

- Add `Eio_mock.Clock` (@talex5 ocaml-multicore/eio#328).
  Control time in tests.

- Add `Buf_read.take_while1` and `skip_while1` (@bikallem ocaml-multicore/eio#309).
  These fail if no characters match.

- Make the type parameter for `Promise.t` covariant (@anmonteiro ocaml-multicore/eio#300).

- Move list functions into a dedicated submodule (@raphael-proust ocaml-multicore/eio#315).

- Direct implementation of `Flow.source_string` (@c-cube ocaml-multicore/eio#317).
  Slightly faster.

Bug fixes:

- `Condition.broadcast` must interlock as well (@haesbaert ocaml-multicore/eio#324).

- Split the reads into no more than 2^32-1 for luv (@haesbaert @talex5 @EduardoRFS ocaml-multicore/eio#343).
  Luv uses a 32 bit int for buffer sizes and wraps if the value passed is too big.

- eio_luv: allow `Net.connect` to be cancelled (@talex5 @nojb ocaml-multicore/eio#311).

- eio_main: Use dedicated log source (@anmonteiro ocaml-multicore/eio#326).

- linux_eio: fix kernel version number in log message (@talex5 @nojb ocaml-multicore/eio#314).

- Account for stack differences in the socketpair test (issue ocaml-multicore/eio#312) (@haesbaert ocaml-multicore/eio#313).

Documentation:

- Add Best Practices section to README (@talex5 ocaml-multicore/eio#299).

- Documentation improvements (@talex5 ocaml-multicore/eio#295 ocaml-multicore/eio#337).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants