Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
1 change: 1 addition & 0 deletions changelog.d/19058.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Remove logcontext problems caused by awaiting raw `deferLater(...)`.
4 changes: 2 additions & 2 deletions synapse/util/task_scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@
class TaskScheduler:
"""
This is a simple task scheduler designed for resumable tasks. Normally,
you'd use `run_in_background` to start a background task or Twisted's
`deferLater` if you want to run it later.
you'd use `run_in_background` to start a background task or `clock.call_later`
if you want to run it later.

The issue is that these tasks stop completely and won't resume if Synapse is
shut down for any reason.
Expand Down
3 changes: 1 addition & 2 deletions tests/rest/admin/test_room.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@

from parameterized import parameterized

from twisted.internet.task import deferLater
from twisted.internet.testing import MemoryReactor

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

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

Expand Down
9 changes: 6 additions & 3 deletions tests/util/test_task_scheduler.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,10 @@
#
from typing import List, Optional, Tuple

from twisted.internet.task import deferLater
from twisted.internet.defer import Deferred
from twisted.internet.testing import MemoryReactor

from synapse.logging.context import make_deferred_yieldable
from synapse.server import HomeServer
from synapse.types import JsonMapping, ScheduledTask, TaskStatus
from synapse.util.clock import Clock
Expand Down Expand Up @@ -87,7 +88,7 @@ async def _sleeping_task(
self, task: ScheduledTask
) -> Tuple[TaskStatus, Optional[JsonMapping], Optional[str]]:
# Sleep for a second
await deferLater(self.reactor, 1, lambda: None)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could have also fixed this by wrapping the deferred with make_deferred_yieldable(...). (see our logcontext docs

ex. await make_deferred_yieldable(deferLater(self.reactor, 1, lambda: None))

Instead, I've opted to replace all of our deferLater usage with more proper clock utilities that handle logcontext rules already.

await self.hs.get_clock().sleep(1)
return TaskStatus.COMPLETE, None, None

def test_schedule_lot_of_tasks(self) -> None:
Expand Down Expand Up @@ -170,8 +171,10 @@ async def _resumable_task(
return TaskStatus.COMPLETE, {"success": True}, None
else:
await self.task_scheduler.update_task(task.id, result={"in_progress": True})
# Create a deferred which we will never complete
incomplete_d: Deferred = Deferred()
# Await forever to simulate an aborted task because of a restart
await deferLater(self.reactor, 2**16, lambda: None)
await make_deferred_yieldable(incomplete_d)
Comment on lines +174 to +177
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the goal is to wait forever, let's actually wait forever instead of 2**16 seconds.

# This should never been called
return TaskStatus.ACTIVE, None, None

Expand Down
Loading