Do not forceTimeOut when a threadGroup has been set

[Raveline]

Force timeout should not be performed if a threadGroup has been provided, since it means we manually handle threads.

[stepcut]

How does that work? Can you actually detect activity and assert your own timeouts? How do you manually handle threads -- it seems like the handler loop built into happstack-server is still the code deciding when to fork new threads, etc.

I am not quite sure why the ThreadGroup support was added to happstack-server. I feel like it was just there to allow for a more graceful shutdown. You first stop accepting new connections, and then you wait for all the currently running requests (threads) to terminate.

I would definitely like to see better support for creating your own scheduler which allows you to create a application specific scheduler that deals with spawning threads, handling timeouts, etc.

[Raveline]

I am not quite sure why the ThreadGroup support was added to happstack-server. I feel like it was just there to allow for a more graceful shutdown. You first stop accepting new connections, and then you wait for all the currently running requests (threads) to terminate.

Well, yes, it's what it's for. But if you run the server in a thread that receives a KillThread exception, you cannot do any operation on the threadGroup with the current state of happstack, since everything gets killed by the forceTimeout. In my use case, I want to be able to run a wait on the groupThread on catching the KillThread exception, to ensure the gracious shutdown.

[stepcut]

If happstack-server does not timeout inactive connections, how do you do it from your application code?

[Raveline]

Oh, nothing sophisticated in my case: just a thread that waits for N seconds before shutting everything in the threadGroup, leaving a period of time sufficient for most handlers to terminate as they should. forceTimeout, however, takes immediate action and doesn't leave a period of grace for handlers to have a chance to terminate.

[arybczak]

@stepcut this PR allows with minimal traction for clients to do what happens in warp, it's just that warp manages the equivalent of waiting for a threadGroup (with a timeout if appropriate) internally: https://github.com/yesodweb/wai/blob/4d2f12c5b5812d64f9dae9872b56c9cc2fd51ec2/warp/Network/Wai/Handler/Warp/Run.hs#L417

[stepcut]

Oh! I misunderstood what this patch was doing. On the surface, this patch seems reasonable. I'll need to ponder it for a bit to be sure.