tornado: Explicitly remove handler when clients disconnect.

This partially reverts 579bdc18f85ea8599c8cf1f53ddb02fd41d97993; it
assumed (based on its documentation) that `on_finish` was called for
all requests, even client-terminated ones.  This is not accurate; it
is only called when the request calls `finish`, which only happens for
successful requests.  This caused every client-closed connection to
leak a handler (ironically, exactly re-introducing the bug previously
fixed in 12a5a3a6e1).

This behaviour was obscured by the development environment's proxy;
see comment added in the previous commit.

Instead of replacing the `clear_handler_by_id` call into
`ClientDescriptor.disconnect_handler`, we instead place it on
`AsyncDjangoHandler.on_connection_close`.  This is more correct for
a few reasons:

- `on_connection_close` will be called if the client goes away during
a request without a client descriptor.  If the handler garbage
collection of handlers runs inside the ClientDescriptor, we leak
handlers.

- `disconnect_handler` also runs when successfully sending an event,
which already calls `on_finish`.  We avoid double-calling
`clear_handler_by_id` by doing it in two clearly exclusive cases,
`on_finish` and `on_connection_close`.

- It combines the creation and garbage collection logic into one
file, decreasing action at a distance which causes memory leaks.

zerver/tornado/handlers.py

@@ -93,13 +93,17 @@ class AsyncDjangoHandler(tornado.web.RequestHandler):
         self._auto_finish = False
 
         # Handler IDs are allocated here, and the handler ID map must
-        # be cleared when the handler finishes its response
+        # be cleared when the handler finishes its response.  See
+        # on_finish and on_connection_close.
         self.handler_id = allocate_handler_id(self)
 
         self._request: Optional[HttpRequest] = None
 
     @override
     def on_finish(self) -> None:
+        # Note that this only runs on _successful_ requests.  If the
+        # client closes the connection, see on_connection_close,
+        # below.
         clear_handler_by_id(self.handler_id)
 
     @override
@@ -214,6 +218,10 @@ class AsyncDjangoHandler(tornado.web.RequestHandler):
         # proxy does not correctly close connections to Tornado when
         # its clients (e.g. `curl`) close their connections.  This
         # code path is thus _unreachable except in production_.
+
+        # If the client goes away, garbage collect the handler (with
+        # attached request information).
+        clear_handler_by_id(self.handler_id)
         client_descriptor = get_descriptor_by_handler_id(self.handler_id)
         if client_descriptor is not None:
             client_descriptor.disconnect_handler(client_closed=True)