Mirrors
/
synapse
mirror of https://github.com/element-hq/synapse.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Add cancellation support to `ReadWriteLock` (#12120) 

Also convert `ReadWriteLock` to use async context managers.

Signed-off-by: Sean Quah <seanq@element.io>
This commit is contained in:
Sean Quah 2022-03-14 18:49:07 +00:00 committed by GitHub
parent 8e5706d144
commit 605d161d7d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 382 additions and 93 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
changelog.d/12120.misc Normal file
View File

@ -0,0 +1 @@
Add support for cancellation to `ReadWriteLock`.

8
synapse/handlers/pagination.py
View File

@ -350,7 +350,7 @@ class PaginationHandler:
"""
self._purges_in_progress_by_room.add(room_id)
try:
with await self.pagination_lock.write(room_id):
async with self.pagination_lock.write(room_id):
await self.storage.purge_events.purge_history(
room_id, token, delete_local_events
)
@ -406,7 +406,7 @@ class PaginationHandler:
room_id: room to be purged
force: set true to skip checking for joined users.
"""
with await self.pagination_lock.write(room_id):
async with self.pagination_lock.write(room_id):
# first check that we have no users in this room
if not force:
joined = await self.store.is_host_joined(room_id, self._server_name)
@ -448,7 +448,7 @@ class PaginationHandler:
room_token = from_token.room_key
with await self.pagination_lock.read(room_id):
async with self.pagination_lock.read(room_id):
(
membership,
member_event_id,
@ -615,7 +615,7 @@ class PaginationHandler:
self._purges_in_progress_by_room.add(room_id)
try:
with await self.pagination_lock.write(room_id):
async with self.pagination_lock.write(room_id):
self._delete_by_id[delete_id].status = DeleteStatus.STATUS_SHUTTING_DOWN
self._delete_by_id[
delete_id

39
synapse/util/async_helpers.py
View File

@ -18,9 +18,10 @@ import collections
import inspect
import itertools
import logging
from contextlib import contextmanager
from contextlib import asynccontextmanager, contextmanager
from typing import (
Any,
AsyncIterator,
Awaitable,
Callable,
Collection,
@ -40,7 +41,7 @@ from typing import (
)
import attr
from typing_extensions import ContextManager, Literal
from typing_extensions import AsyncContextManager, Literal
from twisted.internet import defer
from twisted.internet.defer import CancelledError
@ -491,7 +492,7 @@ class ReadWriteLock:
Example:
with await read_write_lock.read("test_key"):
async with read_write_lock.read("test_key"):
# do some work
"""
@ -514,7 +515,9 @@ class ReadWriteLock:
# Latest writer queued
self.key_to_current_writer: Dict[str, defer.Deferred] = {}
async def read(self, key: str) -> ContextManager:
def read(self, key: str) -> AsyncContextManager:
@asynccontextmanager
async def _ctx_manager() -> AsyncIterator[None]:
new_defer: "defer.Deferred[None]" = defer.Deferred()
curr_readers = self.key_to_current_readers.setdefault(key, set())
@ -522,14 +525,14 @@ class ReadWriteLock:
curr_readers.add(new_defer)
try:
# We wait for the latest writer to finish writing. We can safely ignore
# any existing readers... as they're readers.
# May raise a `CancelledError` if the `Deferred` wrapping us is
# cancelled. The `Deferred` we are waiting on must not be cancelled,
# since we do not own it.
if curr_writer:
await make_deferred_yieldable(curr_writer)
@contextmanager
def _ctx_manager() -> Iterator[None]:
try:
await make_deferred_yieldable(stop_cancellation(curr_writer))
yield
finally:
with PreserveLoggingContext():
@ -538,7 +541,9 @@ class ReadWriteLock:
return _ctx_manager()
async def write(self, key: str) -> ContextManager:
def write(self, key: str) -> AsyncContextManager:
@asynccontextmanager
async def _ctx_manager() -> AsyncIterator[None]:
new_defer: "defer.Deferred[None]" = defer.Deferred()
curr_readers = self.key_to_current_readers.get(key, set())
@ -549,18 +554,22 @@ class ReadWriteLock:
if curr_writer:
to_wait_on.append(curr_writer)
# We can clear the list of current readers since the new writer waits
# We can clear the list of current readers since `new_defer` waits
# for them to finish.
curr_readers.clear()
self.key_to_current_writer[key] = new_defer
await make_deferred_yieldable(defer.gatherResults(to_wait_on))
@contextmanager
def _ctx_manager() -> Iterator[None]:
to_wait_on_defer = defer.gatherResults(to_wait_on)
try:
# Wait for all current readers and the latest writer to finish.
# May raise a `CancelledError` immediately after the wait if the
# `Deferred` wrapping us is cancelled. We must only release the lock
# once we have acquired it, hence the use of `delay_cancellation`
# rather than `stop_cancellation`.
await make_deferred_yieldable(delay_cancellation(to_wait_on_defer))
yield
finally:
# Release the lock.
with PreserveLoggingContext():
new_defer.callback(None)
# `self.key_to_current_writer[key]` may be missing if there was another

395
tests/util/test_rwlock.py
View File

@ -12,8 +12,10 @@
# See the License for the specific language governing permissions and
# limitations under the License.
from typing import AsyncContextManager, Callable, Sequence, Tuple
from twisted.internet import defer
from twisted.internet.defer import Deferred
from twisted.internet.defer import CancelledError, Deferred
from synapse.util.async_helpers import ReadWriteLock
@ -21,87 +23,187 @@ from tests import unittest
class ReadWriteLockTestCase(unittest.TestCase):
def _assert_called_before_not_after(self, lst, first_false):
for i, d in enumerate(lst[:first_false]):
self.assertTrue(d.called, msg="%d was unexpectedly false" % i)
def _start_reader_or_writer(
self,
read_or_write: Callable[[str], AsyncContextManager],
key: str,
return_value: str,
) -> Tuple["Deferred[str]", "Deferred[None]", "Deferred[None]"]:
"""Starts a reader or writer which acquires the lock, blocks, then completes.
for i, d in enumerate(lst[first_false:]):
Args:
read_or_write: A function returning a context manager for a lock.
Either a bound `ReadWriteLock.read` or `ReadWriteLock.write`.
key: The key to read or write.
return_value: A string that the reader or writer will resolve with when
done.
Returns:
A tuple of three `Deferred`s:
* A `Deferred` that resolves with `return_value` once the reader or writer
completes successfully.
* A `Deferred` that resolves once the reader or writer acquires the lock.
* A `Deferred` that blocks the reader or writer. Must be resolved by the
caller to allow the reader or writer to release the lock and complete.
"""
acquired_d: "Deferred[None]" = Deferred()
unblock_d: "Deferred[None]" = Deferred()
async def reader_or_writer():
async with read_or_write(key):
acquired_d.callback(None)
await unblock_d
return return_value
d = defer.ensureDeferred(reader_or_writer())
return d, acquired_d, unblock_d
def _start_blocking_reader(
self, rwlock: ReadWriteLock, key: str, return_value: str
) -> Tuple["Deferred[str]", "Deferred[None]", "Deferred[None]"]:
"""Starts a reader which acquires the lock, blocks, then releases the lock.
See the docstring for `_start_reader_or_writer` for details about the arguments
and return values.
"""
return self._start_reader_or_writer(rwlock.read, key, return_value)
def _start_blocking_writer(
self, rwlock: ReadWriteLock, key: str, return_value: str
) -> Tuple["Deferred[str]", "Deferred[None]", "Deferred[None]"]:
"""Starts a writer which acquires the lock, blocks, then releases the lock.
See the docstring for `_start_reader_or_writer` for details about the arguments
and return values.
"""
return self._start_reader_or_writer(rwlock.write, key, return_value)
def _start_nonblocking_reader(
self, rwlock: ReadWriteLock, key: str, return_value: str
) -> Tuple["Deferred[str]", "Deferred[None]"]:
"""Starts a reader which acquires the lock, then releases it immediately.
See the docstring for `_start_reader_or_writer` for details about the arguments.
Returns:
A tuple of two `Deferred`s:
* A `Deferred` that resolves with `return_value` once the reader completes
successfully.
* A `Deferred` that resolves once the reader acquires the lock.
"""
d, acquired_d, unblock_d = self._start_reader_or_writer(
rwlock.read, key, return_value
)
unblock_d.callback(None)
return d, acquired_d
def _start_nonblocking_writer(
self, rwlock: ReadWriteLock, key: str, return_value: str
) -> Tuple["Deferred[str]", "Deferred[None]"]:
"""Starts a writer which acquires the lock, then releases it immediately.
See the docstring for `_start_reader_or_writer` for details about the arguments.
Returns:
A tuple of two `Deferred`s:
* A `Deferred` that resolves with `return_value` once the writer completes
successfully.
* A `Deferred` that resolves once the writer acquires the lock.
"""
d, acquired_d, unblock_d = self._start_reader_or_writer(
rwlock.write, key, return_value
)
unblock_d.callback(None)
return d, acquired_d
def _assert_first_n_resolved(
self, deferreds: Sequence["defer.Deferred[None]"], n: int
) -> None:
"""Assert that exactly the first n `Deferred`s in the given list are resolved.
Args:
deferreds: The list of `Deferred`s to be checked.
n: The number of `Deferred`s at the start of `deferreds` that should be
resolved.
"""
for i, d in enumerate(deferreds[:n]):
self.assertTrue(d.called, msg="deferred %d was unexpectedly unresolved" % i)
for i, d in enumerate(deferreds[n:]):
self.assertFalse(
d.called, msg="%d was unexpectedly true" % (i + first_false)
d.called, msg="deferred %d was unexpectedly resolved" % (i + n)
)
def test_rwlock(self):
rwlock = ReadWriteLock()
key = object()
key = "key"
ds = [
rwlock.read(key), # 0
rwlock.read(key), # 1
rwlock.write(key), # 2
rwlock.write(key), # 3
rwlock.read(key), # 4
rwlock.read(key), # 5
rwlock.write(key), # 6
self._start_blocking_reader(rwlock, key, "0"),
self._start_blocking_reader(rwlock, key, "1"),
self._start_blocking_writer(rwlock, key, "2"),
self._start_blocking_writer(rwlock, key, "3"),
self._start_blocking_reader(rwlock, key, "4"),
self._start_blocking_reader(rwlock, key, "5"),
self._start_blocking_writer(rwlock, key, "6"),
]
ds = [defer.ensureDeferred(d) for d in ds]
# `Deferred`s that resolve when each reader or writer acquires the lock.
acquired_ds = [acquired_d for _, acquired_d, _ in ds]
# `Deferred`s that will trigger the release of locks when resolved.
release_ds = [release_d for _, _, release_d in ds]
self._assert_called_before_not_after(ds, 2)
# The first two readers should acquire their locks.
self._assert_first_n_resolved(acquired_ds, 2)
with ds[0].result:
self._assert_called_before_not_after(ds, 2)
self._assert_called_before_not_after(ds, 2)
# Release one of the read locks. The next writer should not acquire the lock,
# because there is another reader holding the lock.
self._assert_first_n_resolved(acquired_ds, 2)
release_ds[0].callback(None)
self._assert_first_n_resolved(acquired_ds, 2)
with ds[1].result:
self._assert_called_before_not_after(ds, 2)
self._assert_called_before_not_after(ds, 3)
# Release the other read lock. The next writer should acquire the lock.
self._assert_first_n_resolved(acquired_ds, 2)
release_ds[1].callback(None)
self._assert_first_n_resolved(acquired_ds, 3)
with ds[2].result:
self._assert_called_before_not_after(ds, 3)
self._assert_called_before_not_after(ds, 4)
# Release the write lock. The next writer should acquire the lock.
self._assert_first_n_resolved(acquired_ds, 3)
release_ds[2].callback(None)
self._assert_first_n_resolved(acquired_ds, 4)
with ds[3].result:
self._assert_called_before_not_after(ds, 4)
self._assert_called_before_not_after(ds, 6)
# Release the write lock. The next two readers should acquire locks.
self._assert_first_n_resolved(acquired_ds, 4)
release_ds[3].callback(None)
self._assert_first_n_resolved(acquired_ds, 6)
with ds[5].result:
self._assert_called_before_not_after(ds, 6)
self._assert_called_before_not_after(ds, 6)
# Release one of the read locks. The next writer should not acquire the lock,
# because there is another reader holding the lock.
self._assert_first_n_resolved(acquired_ds, 6)
release_ds[5].callback(None)
self._assert_first_n_resolved(acquired_ds, 6)
with ds[4].result:
self._assert_called_before_not_after(ds, 6)
self._assert_called_before_not_after(ds, 7)
# Release the other read lock. The next writer should acquire the lock.
self._assert_first_n_resolved(acquired_ds, 6)
release_ds[4].callback(None)
self._assert_first_n_resolved(acquired_ds, 7)
with ds[6].result:
pass
# Release the write lock.
release_ds[6].callback(None)
d = defer.ensureDeferred(rwlock.write(key))
self.assertTrue(d.called)
with d.result:
pass
# Acquire and release the write and read locks one last time for good measure.
_, acquired_d = self._start_nonblocking_writer(rwlock, key, "last writer")
self.assertTrue(acquired_d.called)
d = defer.ensureDeferred(rwlock.read(key))
self.assertTrue(d.called)
with d.result:
pass
_, acquired_d = self._start_nonblocking_reader(rwlock, key, "last reader")
self.assertTrue(acquired_d.called)
def test_lock_handoff_to_nonblocking_writer(self):
"""Test a writer handing the lock to another writer that completes instantly."""
rwlock = ReadWriteLock()
key = "key"
unblock: "Deferred[None]" = Deferred()
async def blocking_write():
with await rwlock.write(key):
await unblock
async def nonblocking_write():
with await rwlock.write(key):
pass
d1 = defer.ensureDeferred(blocking_write())
d2 = defer.ensureDeferred(nonblocking_write())
d1, _, unblock = self._start_blocking_writer(rwlock, key, "write 1 completed")
d2, _ = self._start_nonblocking_writer(rwlock, key, "write 2 completed")
self.assertFalse(d1.called)
self.assertFalse(d2.called)
@ -111,5 +213,182 @@ class ReadWriteLockTestCase(unittest.TestCase):
self.assertTrue(d2.called)
# The `ReadWriteLock` should operate as normal.
d3 = defer.ensureDeferred(nonblocking_write())
d3, _ = self._start_nonblocking_writer(rwlock, key, "write 3 completed")
self.assertTrue(d3.called)
def test_cancellation_while_holding_read_lock(self):
"""Test cancellation while holding a read lock.
A waiting writer should be given the lock when the reader holding the lock is
cancelled.
"""
rwlock = ReadWriteLock()
key = "key"
# 1. A reader takes the lock and blocks.
reader_d, _, _ = self._start_blocking_reader(rwlock, key, "read completed")
# 2. A writer waits for the reader to complete.
writer_d, _ = self._start_nonblocking_writer(rwlock, key, "write completed")
self.assertFalse(writer_d.called)
# 3. The reader is cancelled.
reader_d.cancel()
self.failureResultOf(reader_d, CancelledError)
# 4. The writer should take the lock and complete.
self.assertTrue(
writer_d.called, "Writer is stuck waiting for a cancelled reader"
)
self.assertEqual("write completed", self.successResultOf(writer_d))
def test_cancellation_while_holding_write_lock(self):
"""Test cancellation while holding a write lock.
A waiting reader should be given the lock when the writer holding the lock is
cancelled.
"""
rwlock = ReadWriteLock()
key = "key"
# 1. A writer takes the lock and blocks.
writer_d, _, _ = self._start_blocking_writer(rwlock, key, "write completed")
# 2. A reader waits for the writer to complete.
reader_d, _ = self._start_nonblocking_reader(rwlock, key, "read completed")
self.assertFalse(reader_d.called)
# 3. The writer is cancelled.
writer_d.cancel()
self.failureResultOf(writer_d, CancelledError)
# 4. The reader should take the lock and complete.
self.assertTrue(
reader_d.called, "Reader is stuck waiting for a cancelled writer"
)
self.assertEqual("read completed", self.successResultOf(reader_d))
def test_cancellation_while_waiting_for_read_lock(self):
"""Test cancellation while waiting for a read lock.
Tests that cancelling a waiting reader:
* does not cancel the writer it is waiting on
* does not cancel the next writer waiting on it
* does not allow the next writer to acquire the lock before an earlier writer
has finished
* does not keep the next writer waiting indefinitely
These correspond to the asserts with explicit messages.
"""
rwlock = ReadWriteLock()
key = "key"
# 1. A writer takes the lock and blocks.
writer1_d, _, unblock_writer1 = self._start_blocking_writer(
rwlock, key, "write 1 completed"
)
# 2. A reader waits for the first writer to complete.
# This reader will be cancelled later.
reader_d, _ = self._start_nonblocking_reader(rwlock, key, "read completed")
self.assertFalse(reader_d.called)
# 3. A second writer waits for both the first writer and the reader to complete.
writer2_d, _ = self._start_nonblocking_writer(rwlock, key, "write 2 completed")
self.assertFalse(writer2_d.called)
# 4. The waiting reader is cancelled.
# Neither of the writers should be cancelled.
# The second writer should still be waiting, but only on the first writer.
reader_d.cancel()
self.failureResultOf(reader_d, CancelledError)
self.assertFalse(writer1_d.called, "First writer was unexpectedly cancelled")
self.assertFalse(
writer2_d.called,
"Second writer was unexpectedly cancelled or given the lock before the "
"first writer finished",
)
# 5. Unblock the first writer, which should complete.
unblock_writer1.callback(None)
self.assertEqual("write 1 completed", self.successResultOf(writer1_d))
# 6. The second writer should take the lock and complete.
self.assertTrue(
writer2_d.called, "Second writer is stuck waiting for a cancelled reader"
)
self.assertEqual("write 2 completed", self.successResultOf(writer2_d))
def test_cancellation_while_waiting_for_write_lock(self):
"""Test cancellation while waiting for a write lock.
Tests that cancelling a waiting writer:
* does not cancel the reader or writer it is waiting on
* does not cancel the next writer waiting on it
* does not allow the next writer to acquire the lock before an earlier reader
and writer have finished
* does not keep the next writer waiting indefinitely
These correspond to the asserts with explicit messages.
"""
rwlock = ReadWriteLock()
key = "key"
# 1. A reader takes the lock and blocks.
reader_d, _, unblock_reader = self._start_blocking_reader(
rwlock, key, "read completed"
)
# 2. A writer waits for the reader to complete.
writer1_d, _, unblock_writer1 = self._start_blocking_writer(
rwlock, key, "write 1 completed"
)
# 3. A second writer waits for both the reader and first writer to complete.
# This writer will be cancelled later.
writer2_d, _ = self._start_nonblocking_writer(rwlock, key, "write 2 completed")
self.assertFalse(writer2_d.called)
# 4. A third writer waits for the second writer to complete.
writer3_d, _ = self._start_nonblocking_writer(rwlock, key, "write 3 completed")
self.assertFalse(writer3_d.called)
# 5. The second writer is cancelled, but continues waiting for the lock.
# The reader, first writer and third writer should not be cancelled.
# The first writer should still be waiting on the reader.
# The third writer should still be waiting on the second writer.
writer2_d.cancel()
self.assertNoResult(writer2_d)
self.assertFalse(reader_d.called, "Reader was unexpectedly cancelled")
self.assertFalse(writer1_d.called, "First writer was unexpectedly cancelled")
self.assertFalse(
writer3_d.called,
"Third writer was unexpectedly cancelled or given the lock before the first "
"writer finished",
)
# 6. Unblock the reader, which should complete.
# The first writer should be given the lock and block.
# The third writer should still be waiting on the second writer.
unblock_reader.callback(None)
self.assertEqual("read completed", self.successResultOf(reader_d))
self.assertNoResult(writer2_d)
self.assertFalse(
writer3_d.called,
"Third writer was unexpectedly given the lock before the first writer "
"finished",
)
# 7. Unblock the first writer, which should complete.
unblock_writer1.callback(None)
self.assertEqual("write 1 completed", self.successResultOf(writer1_d))
# 8. The second writer should take the lock and release it immediately, since it
# has been cancelled.
self.failureResultOf(writer2_d, CancelledError)
# 9. The third writer should take the lock and complete.
self.assertTrue(
writer3_d.called, "Third writer is stuck waiting for a cancelled writer"
)
self.assertEqual("write 3 completed", self.successResultOf(writer3_d))