Convert inlineCallbacks to async/await

[clokep]

There's a desire to convert code that uses inlineCallbacks to async / await since it makes stack traces nicer and helps with profiling (e.g. #7670).

Some progress on this can be tracked at https://patrick.cloke.us/areweasyncyet/

I've broken down the Synapse code into modules for tracking of this work:

• contrib.cmdclient
• contrib.experiments
• docs.log_contexts Update the logcontext doc #10353
• scripts.synapse_port_db Port synapse_port_db to async/await #6718
• synapse.api async/await is_server_admin #7363 Convert synapse.api to async/await #8031
• synapse.app async/await get_user_id_by_threepid #7620 Convert synapse.app and federation client to async/await. #7868 Convert the main methods run by the reactor to async. #8213
• synapse.appservice Convert appservice to async. #7973 Convert an errant inlineCallbacks in appservice #8207
• synapse.crypto Convert the crypto module to async/await #8003
• synapse.events Convert a synapse.events to async/await #7949
• synapse.federation Convert synapse.federation.transport.server to async #5689 Port federation_server to async/await #6279 Port receipt and read markers to async/wait #6280 Port much of synapse.federation.federation_client to async/await #6840 Cast a coroutine into a Deferred in the federation base #6996 async/await is_server_admin #7363 Convert synapse.app and federation client to async/await. #7868 Convert federation client to async/await. #7975 Fix unawaited coroutine error in tests. #8072
• synapse.groups async/await is_server_admin #7363 Convert groups local and server to async/await #7600 Convert groups and visibility code to async / await. #7951
• synapse.handlers Port room rest handlers to async/await #6275 Port receipt and read markers to async/wait #6280 Port SyncHandler to async/await #6484 Port synapse.handlers.initial_sync to async/await #6496 Port handlers.account_validity to async/await. #6504 Port some of FederationHandler to async/await #6517 Port some admin handlers to async/await #6559 Port much of synapse.handlers.federation to async/await. #6837 Port PresenceHandler to async/await #6991 Convert auth handler to async/await #7261 Convert some of the federation handler methods to async/await. #7338 async/await is_server_admin #7363 Convert the room handler to async/await. #7396 Convert federation handler to async/await. #7459 Convert search code to async/await. #7460 Update the room member handler to use async/await. #7507 Convert sending mail to async/await. #7557 Convert identity handler to async/await. #7561 Convert user directory handler and related classes to async/await #7640 Convert the registration handler to async/await. #7649 Convert the device message and pagination handlers to async/await #7678 Convert the typing handler to async/await. #7679 Convert directory handler to async/await #7727 Convert the appservice handler to async/await. #7775 Convert E2E key and room key handlers to async/await #7851 Convert _base, profile, and _receipts handlers to async/await #7860 Convert device handler to async/await #7871 Convert the message handler to async/await #7884 Convert room list handler to async/await. #7912 Update the auth providers to be async. #7935 Convert presence handler helpers to async/await. #7939 Fix up types and comments that refer to Deferreds. #7945 Remove hacky error handling for inlineDeferreds. #7950
• synapse.http Make the http server handle coroutine-making REST servlets #5475 fix async/await consentresource #5585 Convert the federation agent and related code to async/await. #7874 Convert federation client to async/await. #7975 Convert the SimpleHttpClient to async. #8016 Convert the well known resolver to async #8214
• synapse.logging Fix the trace function for async functions. #7872
• synapse.metrics Convert run_as_background_process inner function into an async function #8032
• synapse.module_api
• synapse.notifier Convert the synapse.notifier module to async/await. #7395
• synapse.push Convert sending mail to async/await. #7557 Convert push to async/await. #7948 Ensure that remove_pusher is always async #7981
• synapse.replication Port replication http server endpoints to async/await #6274 Port synapse.replication.tcp to async/await #6666 Convert replication code to async/await. #7987
• synapse.rest Move rest/admin to use async/await. #6196 Port rest/v1 to async/await #6482 Port rest client v2_alpha to async/await #6483 Convert remote key resource REST layer to async/await. #7020 Convert some of the media REST code to async/await #7110 Convert more of the media code to async/await #7873 Finish converting the media repo code to async / await. #7947 Fix async/await calls for broken media providers. #8027
• synapse.server
• synapse.server_notices async/await is_server_admin #7363 Convert synapse.server_notices to async/await. #7394 Fix some comments and types in service notices #7996
• synapse.spam_checker_api
• synapse.state Convert state resolution to async/await #7942 Add type hints for state. #8140
• synapse.storage Async/await for background updates #6647 Convert delete_url_cache_media to async/await. #7241 async/await is_server_admin #7363 async/await get_user_id_by_threepid #7620 Convert storage layer to async/await. #7963 Convert some of the data store to async #7976 Continue converting store to async/await #8042 Convert additional database stores to async/await #8045 Converts event_federation and registration databases to async/await #8061 Convert tags and metrics databases to async/await #8062 Convert account data, device inbox, and censor events databases to async/await #8063 Convert appservice, group server, profile and more databases to async #8066 Convert devices database to async/await. #8069 Convert the roommember database to async/await. #8070 Convert events worker database to async/await. #8071 Convert stream database to async/await. #8074 Convert pusher databases to async/await. #8075 Convert receipts and events databases to async/await #8076 Convert misc database code to async #8087 Convert some of the general database methods to async #8100 Convert runWithConnection to async. #8121 Do not assume calls to runInteraction return Deferreds. #8133 Convert runInteraction to async/await #8156 Convert simple_select_one and simple_select_one_onecol to async #8162 Convert calls of async database methods to async #8166 Convert additional database methods to async (select list, search, insert_many, delete_*) #8168 Convert simple_update* and simple_select* to async #8173 Convert simple_delete to async/await. #8191 Convert stats and related calls to async/await #8192 Convert state and stream stores and related code to async #8194 Convert a grab bag of database code to async/await #8195 Convert event_push_actions, registration, and roommember datastores to async #8197 Convert additional databases to async/await #8199 Convert additional databases to async/await part 2 #8200 Convert additional databases to async/await part 3 #8201
• synapse.streams Convert streams to async. #8014
• synapse.util Make ObservableDeferred.observe() always return deferred. #6291 Fix stacktraces when using ObservableDeferred and async/await #6836 Convert some util functions to async #8035 Remove the unused inlineCallbacks code-paths in the caching code #8119 Convert ReadWriteLock to async/await. #8202 Convert Clock.sleep to async. #8215
• synapse.visibility Convert groups and visibility code to async / await. #7951
• tests.api Convert additional test-cases to homeserver test case #9396
• tests.appservice
• tests.crypto
• tests.federation
• tests.handlers Convert test cases to use HomeserverTestCase #9377 Convert additional test-cases to homeserver test case #9396
• tests.http
• tests.replication
• tests.rest Convert test cases to use HomeserverTestCase #9377 Convert additional test-cases to homeserver test case #9396
• tests.server
• tests.server_notices
• tests.state
• tests.storage Do not yield on awaitables in tests. #8193 Convert storage test cases to HomeserverTestCase. #9736
• tests.test_federation
• tests.test_server
• tests.test_state
• tests.test_utils Allow for make_awaitable's return value to be re-used. #8261
• tests.test_visibility
• tests.unittest
• tests.util
• tests.utils

[clokep]

I should mention that if anyone is interested in helping out here they can poke me here or in #synapse-dev:matrix.org

[clokep]

To give a bit of a higher level update here:

• Most non-test modules have been converted to use async/await, there's also a few PRs outstanding (notably for runInteraction).
• Most tests need to be switched from using defer.inlineCallbacks style to use get_success().
• There's probably some documentation that needs to be updated.
• It would be nice to see if some ensureDeferred calls could be removed (since that seems to make inlineCallback objects).

[clokep]

The current work on this is done, but I wanted to give another update:

• The vast majority of the code has been converted from inlineCallbacks to async.
• There are a few remaining bits that could be converted, it is probably worth doing those, but at a much lower effort than we spent so far on this.
• Generally methods that return an awaitable are now async and await on other calls instead of passing through awaitables from other calls.
• There are remaining uses of ensureDeferred, which has the unfortunate side-effect of converting async to inlineCallbacks.
• The tests need a lot of updating, in particular the use of tests being defined as inlineCallbacks and then using yield should be replaced with self.get_success, but this requires the test case inheriting from HomeserverTestCase, which not all tests use. This would be a great way to contribute if someone is interested!

[clokep]

It came up that it was never really written down why we were doing this or how we were doing this. I'll attempt to document what I remember:

Why convert inlineCallbacks to async/await:

• inlineCallbacks mangle stack traces pretty badly, async/await should improve the stack traces for profiling and exceptions (e.g. in Sentry).
• async/await is more modern looking and better understood by people who don't have deep knowledge of Twisted.
• Since async/await is a language feature it has better support from other packages, static analyzers, tools, etc.
• async functions can provide better type hints for return values.
• There's some thought that this might have a small performance boost (although I don't think we ever saw any).

Rules for convert from inlineCallbacks to async/await:

The result of calling an async function is an Awaitable, the result of calling an inlineCallbacks function is a Deferred. async functions use await internally to wait for another Awaitable, inlineCallbacks use yield internally to wait for another Deferred.

• You can await a Deferred (since it is also an Awaitable).
• You cannot yield an Awaitable.
• You can convert an Awaitable into a Deferred via defer.ensureDeferred.
• Note that you can yield on a non-Deferred (and it just immediately continues), but calling await on a non-Awaitable is a runtime error.
• Twisted APIs still expect Deferreds.

Methodology for converting from inlineCallbacks to async/await:

Since you can await a Deferred the easiest way to do this is to start at the outer layers and work inward. By doing this you end up with async functions which call into code which returns Deferreds, but this is fine. For Synapse we converted things via:

1. The REST layer.
2. The handler layer.
3. The database layer.

In order to avoid doing an entire layer at once you want to start with the modules which are called into the least (preferably only via the layer above!). If there are other callers which have not yet been converted, the call-site is modified to wrap the returned Awaitable with ensureDeferred.

The REST layer needed some magic since this needs to integrate into Twisted, see _AsyncResource and sub-classes, in particular this:

1. Overrides render (which is a Twisted API from IResource).
   1. Calls the async function with defer.ensureDeferred to ensure it gets scheduled with the reactor.
   2. Returns NOT_DONE_YET so that Twisted doesn't close the connection.
2. It then searches for a method called _async_render_<HTTP METHOD> and calls it.
3. If that is also async it awaits it.
4. Finally it sends the response.

We also had many places which were undecorated functions which returned a Deferred via calling something else. While doing this conversion we updated those to be async and then internally await the called function, for clarity.

This also involved updating the tests to match the type as well (i.e. if a function was made async and we mock that function somewhere, the mock should also be async).

[clokep]

As this isn't actively being worked on I'm not sure having a tracking issue for this makes sense anymore. The https://patrick.cloke.us/areweasyncyet/ page was updated as of yesterday (and is easy enough to keep up-to-date).

I think most of the remaining bits are converting the tests to use HomeserverTestCase + get_success instead of marking the test as inlineCallbacks and yield-ing each Deferred. This is tedious, although not particularly hard. It also isn't particularly urgent since the gains from it are minor.

I think most of the remaining uses in the synapse package probably need to be kept in order to interface with Twisted APIs, although there might be a few more that could go away.