Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Generalise the @cancellable annotation so it can be used on functions other than just servlet methods. #13662

Merged
merged 6 commits into from
Aug 31, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion synapse/federation/transport/server/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

from synapse.api.errors import Codes, FederationDeniedError, SynapseError
from synapse.api.urls import FEDERATION_V1_PREFIX
from synapse.http.server import HttpServer, ServletCallback, is_method_cancellable
from synapse.http.server import HttpServer, ServletCallback
from synapse.http.servlet import parse_json_object_from_request
from synapse.http.site import SynapseRequest
from synapse.logging.context import run_in_background
Expand All @@ -34,6 +34,7 @@
whitelisted_homeserver,
)
from synapse.types import JsonDict
from synapse.util.cancellation import is_method_cancellable
from synapse.util.ratelimitutils import FederationRateLimiter
from synapse.util.stringutils import parse_and_validate_server_name

Expand Down
45 changes: 1 addition & 44 deletions synapse/http/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@
Optional,
Pattern,
Tuple,
TypeVar,
Union,
)

Expand Down Expand Up @@ -64,6 +63,7 @@
from synapse.logging.opentracing import active_span, start_active_span, trace_servlet
from synapse.util import json_encoder
from synapse.util.caches import intern_dict
from synapse.util.cancellation import is_method_cancellable
from synapse.util.iterutils import chunk_seq

if TYPE_CHECKING:
Expand Down Expand Up @@ -94,9 +94,6 @@
HTTP_STATUS_REQUEST_CANCELLED = 499


F = TypeVar("F", bound=Callable[..., Any])


_cancellable_method_names = frozenset(
{
# `RestServlet`, `BaseFederationServlet` and `BaseFederationServerServlet`
Expand All @@ -118,46 +115,6 @@
)


def cancellable(method: F) -> F:
"""Marks a function as cancellable.

Servlet methods with this decorator will be cancelled if the client disconnects before we
finish processing the request.

Although this annotation is particularly useful for servlet methods, it's also
useful for intermediate functions, where it documents the fact that the function has
been audited for cancellation safety and needs to preserve that.
This then simplifies auditing new functions that call those same intermediate
functions.

During cancellation, `Deferred.cancel()` will be invoked on the `Deferred` wrapping
the method. The `cancel()` call will propagate down to the `Deferred` that is
currently being waited on. That `Deferred` will raise a `CancelledError`, which will
propagate up, as per normal exception handling.

Before applying this decorator to a new function, you MUST recursively check
that all `await`s in the function are on `async` functions or `Deferred`s that
handle cancellation cleanly, otherwise a variety of bugs may occur, ranging from
premature logging context closure, to stuck requests, to database corruption.

See the documentation page on Cancellation for more information.

Usage:
class SomeServlet(RestServlet):
@cancellable
async def on_GET(self, request: SynapseRequest) -> ...:
...
"""

method.cancellable = True # type: ignore[attr-defined]
return method


def is_method_cancellable(method: Callable[..., Any]) -> bool:
"""Checks whether a servlet method has the `@cancellable` flag."""
return getattr(method, "cancellable", False)


def return_json_error(
f: failure.Failure, request: SynapseRequest, config: Optional[HomeServerConfig]
) -> None:
Expand Down
3 changes: 2 additions & 1 deletion synapse/replication/http/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,13 @@

from synapse.api.errors import HttpResponseException, SynapseError
from synapse.http import RequestTimedOutError
from synapse.http.server import HttpServer, is_method_cancellable
from synapse.http.server import HttpServer
from synapse.http.site import SynapseRequest
from synapse.logging import opentracing
from synapse.logging.opentracing import trace_with_opname
from synapse.types import JsonDict
from synapse.util.caches.response_cache import ResponseCache
from synapse.util.cancellation import is_method_cancellable
from synapse.util.stringutils import random_string

if TYPE_CHECKING:
Expand Down
3 changes: 2 additions & 1 deletion synapse/rest/client/room.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
)
from synapse.api.filtering import Filter
from synapse.events.utils import format_event_for_client_v2
from synapse.http.server import HttpServer, cancellable
from synapse.http.server import HttpServer
from synapse.http.servlet import (
ResolveRoomIdMixin,
RestServlet,
Expand All @@ -57,6 +57,7 @@
from synapse.streams.config import PaginationConfig
from synapse.types import JsonDict, StreamToken, ThirdPartyInstanceID, UserID
from synapse.util import json_decoder
from synapse.util.cancellation import cancellable
from synapse.util.stringutils import parse_and_validate_server_name, random_string

if TYPE_CHECKING:
Expand Down
56 changes: 56 additions & 0 deletions synapse/util/cancellation.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
# Copyright 2022 The Matrix.org Foundation C.I.C.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
from typing import Any, Callable, TypeVar

F = TypeVar("F", bound=Callable[..., Any])


def cancellable(method: F) -> F:
"""Marks a function as cancellable.

Servlet methods with this decorator will be cancelled if the client disconnects before we
finish processing the request.

Although this annotation is particularly useful for servlet methods, it's also
useful for intermediate functions, where it documents the fact that the function has
been audited for cancellation safety and needs to preserve that.
This then simplifies auditing new functions that call those same intermediate
functions.

During cancellation, `Deferred.cancel()` will be invoked on the `Deferred` wrapping
the method. The `cancel()` call will propagate down to the `Deferred` that is
currently being waited on. That `Deferred` will raise a `CancelledError`, which will
propagate up, as per normal exception handling.

Before applying this decorator to a new function, you MUST recursively check
that all `await`s in the function are on `async` functions or `Deferred`s that
handle cancellation cleanly, otherwise a variety of bugs may occur, ranging from
premature logging context closure, to stuck requests, to database corruption.

See the documentation page on Cancellation for more information.

Usage:
class SomeServlet(RestServlet):
@cancellable
async def on_GET(self, request: SynapseRequest) -> ...:
...
"""

method.cancellable = True # type: ignore[attr-defined]
return method


def is_method_cancellable(method: Callable[..., Any]) -> bool:
reivilibre marked this conversation as resolved.
Show resolved Hide resolved
"""Checks whether a servlet method has the `@cancellable` flag."""
return getattr(method, "cancellable", False)
3 changes: 2 additions & 1 deletion tests/federation/transport/server/test__base.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@
from synapse.api.errors import Codes
from synapse.federation.transport.server import BaseFederationServlet
from synapse.federation.transport.server._base import Authenticator, _parse_auth_header
from synapse.http.server import JsonResource, cancellable
from synapse.http.server import JsonResource
from synapse.server import HomeServer
from synapse.types import JsonDict
from synapse.util.cancellation import cancellable
from synapse.util.ratelimitutils import FederationRateLimiter

from tests import unittest
Expand Down
2 changes: 1 addition & 1 deletion tests/http/test_servlet.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@
from unittest.mock import Mock

from synapse.api.errors import Codes, SynapseError
from synapse.http.server import cancellable
from synapse.http.servlet import (
RestServlet,
parse_json_object_from_request,
Expand All @@ -28,6 +27,7 @@
from synapse.rest.client._base import client_patterns
from synapse.server import HomeServer
from synapse.types import JsonDict
from synapse.util.cancellation import cancellable

from tests import unittest
from tests.http.server._base import test_disconnect
Expand Down
3 changes: 2 additions & 1 deletion tests/replication/http/test__base.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,12 @@
from twisted.web.server import Request

from synapse.api.errors import Codes
from synapse.http.server import JsonResource, cancellable
from synapse.http.server import JsonResource
from synapse.replication.http import REPLICATION_PREFIX
from synapse.replication.http._base import ReplicationEndpoint
from synapse.server import HomeServer
from synapse.types import JsonDict
from synapse.util.cancellation import cancellable

from tests import unittest
from tests.http.server._base import test_disconnect
Expand Down
2 changes: 1 addition & 1 deletion tests/test_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@
DirectServeJsonResource,
JsonResource,
OptionsResource,
cancellable,
)
from synapse.http.site import SynapseRequest, SynapseSite
from synapse.logging.context import make_deferred_yieldable
from synapse.types import JsonDict
from synapse.util import Clock
from synapse.util.cancellation import cancellable

from tests import unittest
from tests.http.server._base import test_disconnect
Expand Down