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

Long-running application and closed connections #196

Open
snoyberg opened this issue Nov 7, 2013 · 11 comments
Open

Long-running application and closed connections #196

snoyberg opened this issue Nov 7, 2013 · 11 comments

Comments

@snoyberg
Copy link
Member

snoyberg commented Nov 7, 2013

I believe this is the cause of snoyberg/keter#29. It's not necessarily a bug in Warp, but it's a design point that requires some consideration. Consider the following interaction between an HTTP client, Warp, and a WAI application:

  1. Client connects to server.
  2. Warp parses request and passes it to Application.
  3. Application does something that takes a long period of time. Warp currently disables timeout handling to specifically allow for these kinds of long-polling situations.
  4. Client meanwhile gets bored and cancels the connection. Warp will not notice that the connection has been closed until it tries to send data to the client, which will only happen after the Application returns a Response.
  5. Application completes its long-running action. It now goes to (for example) a message queue it has for messages to be sent to the client, takes a message off the queue, forms a Response from it, and returns that Response to Warp.
  6. Warp tries to send that Response to the client, but the connection has already been closed, so the data is thrown away.

What we'd theoretically want is some kind of a notification in the Application when the connection is closed. Either the Application could do some polling to determine that, or an async exception could be thrown whenever the connection is closed. One idea would be for WAI to include a new field, clientIsListener :: IO Bool. That field wouldn't necessarily be accurate, so perhaps a Maybe Bool response would be more accurate.

Another possibility is to say that the current behavior is acceptable, and it's the responsibility of the Application to institute some kind of a heartbeat.

Pinging @chrisdone and @jwiegley.

@snoyberg
Copy link
Member Author

snoyberg commented Nov 7, 2013

Chris mentioned isConnected. After a little research, the function seems to be a lie, which isn't surprising given the fact that the socket API doesn't really expose a way to see if a socket is still connected:

{-# LANGUAGE OverloadedStrings #-}
import           Control.Concurrent   (threadDelay)
import           Control.Exception    (bracket)
import           Control.Monad        (forever)
import           Data.Conduit.Network (bindPort)
import           Network.Socket       (accept, isConnected, sClose)

main :: IO ()
main = bracket (bindPort 4000 "*") sClose $ \listener -> forever $ do
    (incoming, _) <- accept listener
    forever $ do
        isConnected incoming >>= print
        threadDelay 1000000

@chrisdone
Copy link

Submitted pull request to network to document this: haskell/network#116

@exFalso
Copy link

exFalso commented Jan 25, 2015

Bump, this would be nice to have. I'm implementing a sockjs wai middleware and some tests in the specification require the server to close the connection if a poll request is aborted - however there is no way to detect this in warp/wai now.

If i understand it correctly one can detect the socket disconnect by checking whether recv returns a zero length bytestring. The only problem is that whatever thread is checking this needs to eagerly buffer incoming data just to see whether the connection has been dropped while the app is processing. Not fun.

However the user could provide a maximum buffer size - then the server could detect disconnects as long as unprocessed incoming data fits in this buffer. I think at least for polling protocols this would be a good enough "best effort" solution - polling requests are pretty much empty after all

@reiddraper
Copy link
Contributor

I just ran into this when using a ResponseStream response type. I'm using server-sent events, and in a particular case, my application would never try to write to the socket. This ended up leaking resources. That being said, once I realized this, it was easy enough to simply send a heartbeat message every sixty-seconds or so. Trying to send this message on a closed socket appropriately throws an exception, and my bracket 'release' handler gets called.

@anacrolix
Copy link

I'm not sure what the state of this issue is. Was something implemented for this purpose?

@robinp
Copy link

robinp commented Feb 28, 2019

Current behavior seems to still not notice that the connection was gone.

The post http://stefan.buettcher.org/cs/conn_closed.html describes a way to get async notification about a closed connection: select on the connection, and if it says data available, do a peek-only + dontwaint recv to see if there's actual data available or the connection was closed (in case bytes available is zero).

It would be nice if warp supported excepting the application early, but at least added to the docs.

In the mean time, some workarounds to consider, depending on $things:

  • Try streaming back pings as @reiddraper advised.
  • Send a separate control message from the client to notify about the cancellation, and kill the related response thread on the app-level.
  • Set a client cookie, auto-kill previous request from same client.

Could this be reopened? @snoyberg

@snoyberg
Copy link
Member Author

snoyberg commented Mar 3, 2019

I can reopen, but I'm not planning on looking into this myself. If you're interested in taking a stab at this, feel free. However, given the likely dangerous nature of introducing more asynchronous exceptions at surprising points of the codebase, I can't guarantee anything will get merged in.

@snoyberg snoyberg reopened this Mar 3, 2019
@win-t
Copy link

win-t commented Dec 15, 2019

Hi, I'm trying to build a simple web server with WARP.
I coming from golang, in golang they use context to notify if a request is canceled or not.
I look into the implementation here, they use a separate thread (goroutine) to read from the socket, if the socket returns EOF it means that the socket is done and close the context.Context so other thread can abort whatever they doing.
I think we can use the same logic in warp

@asheshambasta
Copy link

I have a question about this: how would middlewares act then? For example, a client terminates the connection midway while transmitting the request body. If I'm getting this correctly, anything that consumes the request body will never terminate? Will the body logger ever terminate?

I've been debugging a memory leak in our API gateway service for months now, without any success. And I just want to rule out the issue having an external source.

@nh2
Copy link

nh2 commented Apr 23, 2023

Here is how other programs handle this:

  • nc detect disconnects, as strace shows:

    // nc waits for either user input, or something from the remote side
    poll([{fd=0</dev/pts/25>, events=POLLIN}, {fd=4<socket:[19333096]>, events=0}, {fd=4<socket:[19333096]>, events=POLLIN}, {fd=1</dev/pts/25>, events=0}], 4, -1) = 1 ([{fd=4, revents=POLLIN}])
    
    // the remote side said something, nc checks for new data
    read(4<socket:[19333096]>, "", 16384)   = 0
  • nginx will log HTTP 499 Client Closed Request immediately when the client disconnects (while nginx is waiting on a reply from an upstream server).

    This is how it does it with epoll:

    // nginx tells epoll that it wants to be informed about `EPOLLRDHUP` (explanation below).
    epoll_ctl(7<anon_inode:[eventpoll]>, EPOLL_CTL_MOD, 5<socket:[19552907]>, {events=EPOLLIN|EPOLLOUT|EPOLLRDHUP|EPOLLET, data={u32=2167254769, u64=93877267443441}}) = 0
    
    epoll_wait(7<anon_inode:[eventpoll]>, 
      // Blocks here until client connects.
    
      // The returned `EPOLLRDHUP` indicates
      // > Stream socket peer closed connection, or shut down writing half of connection.
    
      [{events=EPOLLIN|EPOLLOUT|EPOLLRDHUP, data={u32=2167254769, u64=93877267443441}}], 512, 60000) = 1
    
    // Check for connection error; it returns `0` which indicates there was no error.
    getsockopt(5<socket:[19466271]>, SOL_SOCKET, SO_ERROR, [0], [4]) = 0
    
    // closes connection to upstream:
    close(10<socket:[19466273]>) = 0
    
    // logs disconnect:
    write(3</dev/pts/26>, "127.0.0.1 - - [23/Apr/2023:03:40:10 +0200] \"GET / HTTP/1.1\" 499 0 \"-\" \"curl/7.86.0\"\n", 84) = 84
    
    // closes socket to client:
    close(5<socket:[19466271]>) = 0

It looks like the GHC IO Manager does not yet have a way to use EPOLLRDHUP.

Perhaps there's a different way we can detect disconnects, e.g. with a recv() in a thread?

@robinp
Copy link

robinp commented Nov 29, 2023

I created a proof-of-concept using the networking primitives available in userland so far: robinp@31a89b4

It is not very nice, but not too horrible either. When testing locally, got the job done. Maybe it sparks some ideas.

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

9 participants