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

Exposing peer-closed events via GHC's event manager #209

Closed
bgamari opened this issue Sep 21, 2023 · 7 comments
Closed

Exposing peer-closed events via GHC's event manager #209

bgamari opened this issue Sep 21, 2023 · 7 comments
Labels
withdrawn Withdrawn by proposer

Comments

@bgamari
Copy link

bgamari commented Sep 21, 2023

Some platforms (e.g. Linux via EPOLLRDHUP) can provide notifications to socket users when the socket's peer closes the read side of their connection. This information can sometimes be exploited by server applications to abort on-going request handlers when it is known that the client will no longer be able to receive the result.

Here we propose to expose such events via GHC's event manager. Specifically, we propose exposing:

-- | The peer of a socket has closed the read side of its connection.
evtPeerClosed :: Event

from GHC.Event.Internal.Types, GHC.Event.Manager, and GHC.Event. This can be used with the existing GHC.Event.Manager.registerFd function to register for peer-closed events.

Furthermore, we propose exposing the following via GHC.Event.Manager:

threadWaitPeerClosed :: Fd -> IO ()
threadWaitPeerClosedSTM :: Fd -> IO (STM (), IO ())

Attempting to use this operation on a non-supported platform will result in an unsupportedOperation exception.

Querying backend support

This proposal would break the current property that all IO manager backends support all event types (which currently include only the ubiquitously-supported evtRead, evtWrite, and evtClose). As exception handling can be expensive, I suggest that we offer a new operation:

-- | Query whether a particular 'Event' is supported by the current backend.
eventSupported :: EventManager -> Event -> IO Bool

Implementation

An implementation of this can be found in https://gitlab.haskell.org/ghc/ghc/-/merge_requests/11080; this implementation currently only covers Linux's EPOLLRDHUP although we expect that it could also be implemented on KQueue-based platforms. A user can be found in yesodweb/wai#937.

@Ericson2314
Copy link
Contributor

I would prefer that none of GHC.Event.* live in base.

Perhaps this is a good time to think of a better home for GHC.Event.*, make it a deprecated reexport in base, and just add new stuff in the new home.

@tomjaguarpaw
Copy link
Member

It seems to me this is a good candidate for exposing through ghc-experimental, isn't it? When it's had solid uptake and been proved out in practice then we can consider exposing it from base (if we haven't decided, in the meantime, that event stuff belongs in a new home, per @Ericson2314).

@bgamari
Copy link
Author

bgamari commented Sep 21, 2023

I would prefer that none of GHC.Event.* live in base.

FWIW, I would also prefer this but I am reluctant to hold up this small change on what is a rather significant change with non-obvious design decisions. Let's consider this orthogonally.

It seems to me this is a good candidate for exposing through ghc-experimental, isn't it?

This would be fine by me.

@Ericson2314
Copy link
Contributor

Ericson2314 commented Sep 22, 2023

Sure ghc-experimental is fine for now, better than base.

@Bodigrim
Copy link
Collaborator

@bgamari how would you like to proceed here? I see that the rebased https://gitlab.haskell.org/ghc/ghc/-/merge_requests/11080/diffs implements the necessary functionality in ghc-internal, so presumably we no longer need to expose it through base?

@Bodigrim
Copy link
Collaborator

@bgamari it would be great to hear from you, otherwise I'll close as abandoned in two weeks.

@bgamari
Copy link
Author

bgamari commented Apr 26, 2024

Indeed I believe this should no longer be necessary. Thanks for the ping, @Bodigrim !

@bgamari bgamari closed this as completed Apr 26, 2024
@Bodigrim Bodigrim added the withdrawn Withdrawn by proposer label Apr 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
withdrawn Withdrawn by proposer
Projects
None yet
Development

No branches or pull requests

4 participants