Skip to content

Commit 0417296

Browse files
Remove logcontext problems caused by awaiting raw deferLater(...) (#19058)
This is a normal problem where we `await` a deferred without wrapping it in `make_deferred_yieldable(...)`. But I've opted to replace the usage of `deferLater` with something more standard for the Synapse codebase. Part of #18905 It's unclear why we're only now seeing these failures happen with the changes from #19057 Example failures seen in https://github.com/element-hq/synapse/actions/runs/18477454390/job/52645183606?pr=19057 ``` builtins.AssertionError: Expected `looping_call` callback from the reactor to start with the sentinel logcontext but saw task-_resumable_task-0-IBzAmHUoepQfLnEA. In other words, another task shouldn't have leaked their logcontext to us. ```
1 parent 1823287 commit 0417296

File tree

4 files changed

+10
-7
lines changed

4 files changed

+10
-7
lines changed

changelog.d/19058.misc

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Remove logcontext problems caused by awaiting raw `deferLater(...)`.

synapse/util/task_scheduler.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,8 @@
5353
class TaskScheduler:
5454
"""
5555
This is a simple task scheduler designed for resumable tasks. Normally,
56-
you'd use `run_in_background` to start a background task or Twisted's
57-
`deferLater` if you want to run it later.
56+
you'd use `run_in_background` to start a background task or `clock.call_later`
57+
if you want to run it later.
5858
5959
The issue is that these tasks stop completely and won't resume if Synapse is
6060
shut down for any reason.

tests/rest/admin/test_room.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727

2828
from parameterized import parameterized
2929

30-
from twisted.internet.task import deferLater
3130
from twisted.internet.testing import MemoryReactor
3231

3332
import synapse.rest.admin
@@ -1163,7 +1162,7 @@ def test_delete_same_room_twice(self) -> None:
11631162
# Mock PaginationHandler.purge_room to sleep for 100s, so we have time to do a second call
11641163
# before the purge is over. Note that it doesn't purge anymore, but we don't care.
11651164
async def purge_room(room_id: str, force: bool) -> None:
1166-
await deferLater(self.hs.get_reactor(), 100, lambda: None)
1165+
await self.hs.get_clock().sleep(100)
11671166

11681167
self.pagination_handler.purge_room = AsyncMock(side_effect=purge_room) # type: ignore[method-assign]
11691168

tests/util/test_task_scheduler.py

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,10 @@
2020
#
2121
from typing import Optional
2222

23-
from twisted.internet.task import deferLater
23+
from twisted.internet.defer import Deferred
2424
from twisted.internet.testing import MemoryReactor
2525

26+
from synapse.logging.context import make_deferred_yieldable
2627
from synapse.server import HomeServer
2728
from synapse.types import JsonMapping, ScheduledTask, TaskStatus
2829
from synapse.util.clock import Clock
@@ -87,7 +88,7 @@ async def _sleeping_task(
8788
self, task: ScheduledTask
8889
) -> tuple[TaskStatus, Optional[JsonMapping], Optional[str]]:
8990
# Sleep for a second
90-
await deferLater(self.reactor, 1, lambda: None)
91+
await self.hs.get_clock().sleep(1)
9192
return TaskStatus.COMPLETE, None, None
9293

9394
def test_schedule_lot_of_tasks(self) -> None:
@@ -170,8 +171,10 @@ async def _resumable_task(
170171
return TaskStatus.COMPLETE, {"success": True}, None
171172
else:
172173
await self.task_scheduler.update_task(task.id, result={"in_progress": True})
174+
# Create a deferred which we will never complete
175+
incomplete_d: Deferred = Deferred()
173176
# Await forever to simulate an aborted task because of a restart
174-
await deferLater(self.reactor, 2**16, lambda: None)
177+
await make_deferred_yieldable(incomplete_d)
175178
# This should never been called
176179
return TaskStatus.ACTIVE, None, None
177180

0 commit comments

Comments
 (0)