tornado: Consistently clear handler, via on_finish.

initialize() is called on every request, and stored the
`RequestHandler` (and thus `HTTPServerRequest`) in a global shared
dict.  However, the object is only removed from that structure if the
request was successful.  This means that failed requests (such as 405
Method Not Allowed) leaked `RequestHandler`s and
`HTTPServerRequest`s.

Move the cleanup to `on_finish`, which is called at the close of all
requests, async and not, successful or not.

zerver/tornado/event_queue.py

@@ -48,12 +48,7 @@ from zerver.middleware import async_request_timer_restart
 from zerver.models import CustomProfileField
 from zerver.tornado.descriptors import clear_descriptor_by_handler_id, set_descriptor_by_handler_id
 from zerver.tornado.exceptions import BadEventQueueIdError
-from zerver.tornado.handlers import (
-    clear_handler_by_id,
-    finish_handler,
-    get_handler_by_id,
-    handler_stats_string,
-)
+from zerver.tornado.handlers import finish_handler, get_handler_by_id, handler_stats_string
 
 # The idle timeout used to be a week, but we found that in that
 # situation, queues from dead browser sessions would grow quite large
@@ -283,7 +278,6 @@ class ClientDescriptor:
     def disconnect_handler(self, client_closed: bool = False) -> None:
         if self.current_handler_id:
             clear_descriptor_by_handler_id(self.current_handler_id)
-            clear_handler_by_id(self.current_handler_id)
             if client_closed:
                 logging.info(
                     "Client disconnected for queue %s (%s via %s)",

zerver/tornado/handlers.py

@@ -98,6 +98,10 @@ class AsyncDjangoHandler(tornado.web.RequestHandler):
 
         self._request: Optional[HttpRequest] = None
 
+    @override
+    def on_finish(self) -> None:
+        clear_handler_by_id(self.handler_id)
+
     @override
     def __repr__(self) -> str:
         descriptor = get_descriptor_by_handler_id(self.handler_id)
@@ -184,10 +188,6 @@ class AsyncDjangoHandler(tornado.web.RequestHandler):
                 # For normal/synchronous requests that don't end up
                 # long-polling, we just need to write the HTTP
                 # response that Django prepared for us via Tornado.
-
-                # Mark this handler ID as finished for Zulip's own tracking.
-                clear_handler_by_id(self.handler_id)
-
                 assert isinstance(response, HttpResponse)
                 await self.write_django_response_as_tornado_response(response)
         finally: