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

Clear up confusion between FLOW close, shutdown #16

Closed
djs55 opened this issue Jun 22, 2016 · 1 comment
Closed

Clear up confusion between FLOW close, shutdown #16

djs55 opened this issue Jun 22, 2016 · 1 comment

Comments

@djs55
Copy link
Member

djs55 commented Jun 22, 2016

The Unix API has

  • close: means to release all resources and drop in-flight data
  • shutdown_write: means to signal EOF to the remote, while the connection stays up and data can be read
  • shutdown_read: means to locally ignore any more pending reads

while the Mirage API was recently clarified to follow the (sensible) behaviour of the tcpip stack:

  • close: means to signal EOF to the remote (and it provides a thread which allows handshaking with a close sent from the other end)

The Mirage tcpip stack will clean up resources after the close calls have been acknowledged, taking into account TIME_WAIT states etc (like a Unix kernel would). There is no "socket" "file descriptor" or "socket buffers" which need to be freed separately by the caller since there is no socket layer.

The current proxy loop has 2 threads, one for each direction which so something like:

  • read until Eof
  • shutdown_read: this is a bit pointless
  • shutdown_write: this sends an Eof to the remote

when both threads have finished the proxy returns, and the caller can release any associated resources by calling close. This misunderstanding has lead to strange implementations.

We should drop the shutdown_read (pointless) and convert the shutdown_write into an asynchronous call to close. This means that implementations other than tcpip which are currently calling functions like Lwt_unix.close in close will need to stop doing this and use some other function (disconnect?) instead.

djs55 added a commit to djs55/mirage-flow that referenced this issue Jun 22, 2016
This follows the clarification that `V1_LWT.close` follows the behaviour
of the Mirage tcp/ip stack i.e. it signals a "shutdown" to the remote
(like sending an `Eof`) while leaving the resources allocated and
the connection up. Implementations which associate extra resources with
a connection (such as a file descriptor or socket buffers) need provide
some other cleanup method (e.g. a `disconnect` function)

Fixes mirage#16

Signed-off-by: David Scott <dave@recoil.org>
@hannesm
Copy link
Member

hannesm commented Dec 17, 2023

we have merged #48 now, which adds a shutdown :)

@hannesm hannesm closed this as completed Dec 17, 2023
hannesm added a commit to hannesm/opam-repository that referenced this issue Dec 17, 2023
…rs (4.0.0)

CHANGES:

- Add ``shutdown : flow -> [ `read | `write | `read_write ] -> unit Lwt.t``
  (@djs55 @hannesm mirage/mirage-flow#16 mirage/mirage-flow#18 mirage/mirage-flow#48)
- Remove SHUTDOWNABLE signature (@djs55 mirage/mirage-flow#17, rebased into mirage/mirage-flow#48)
hannesm added a commit to hannesm/opam-repository that referenced this issue Dec 19, 2023
…rs (4.0.0)

CHANGES:

- Redefine `close` semantics, which no longer is a `` shutdown `read_write ``
  (mirage/mirage-flow#49 @hannesm)
- Add ``shutdown : flow -> [ `read | `write | `read_write ] -> unit Lwt.t``
  (@djs55 @hannesm mirage/mirage-flow#16 mirage/mirage-flow#18 mirage/mirage-flow#48)
- Remove SHUTDOWNABLE signature (@djs55 mirage/mirage-flow#17, rebased into mirage/mirage-flow#48)
hannesm added a commit to hannesm/opam-repository that referenced this issue Dec 19, 2023
…rs (4.0.0)

CHANGES:

- Redefine `close` semantics, which no longer is a `` shutdown `read_write ``
  (mirage/mirage-flow#49 @hannesm)
- Add ``shutdown : flow -> [ `read | `write | `read_write ] -> unit Lwt.t``
  (@djs55 @hannesm mirage/mirage-flow#16 mirage/mirage-flow#18 mirage/mirage-flow#48)
- Remove SHUTDOWNABLE signature (@djs55 mirage/mirage-flow#17, rebased into mirage/mirage-flow#48)
nberth pushed a commit to nberth/opam-repository that referenced this issue Jun 18, 2024
…rs (4.0.0)

CHANGES:

- Redefine `close` semantics, which no longer is a `` shutdown `read_write ``
  (mirage/mirage-flow#49 @hannesm)
- Add ``shutdown : flow -> [ `read | `write | `read_write ] -> unit Lwt.t``
  (@djs55 @hannesm mirage/mirage-flow#16 mirage/mirage-flow#18 mirage/mirage-flow#48)
- Remove SHUTDOWNABLE signature (@djs55 mirage/mirage-flow#17, rebased into mirage/mirage-flow#48)
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

No branches or pull requests

2 participants