mirror of
https://github.com/element-hq/synapse.git
synced 2026-11-14 16:09:34 +00:00
Sid Karunaratne
86c96382b2
Fix a bug introduced in Synapse v1.146.0 where failed remote media downloads and URL previews would leave behind empty files in the media store. Regressed in #19204 which restructured `MediaStorage.store_into_file(...)` to support running without a local media store. As part of that, the `yield` (where the caller writes into the file) was moved outside of the `try`/`except` that cleans up the file on failure. Since `store_into_file(...)` is an `asynccontextmanager`, an exception raised by the caller is thrown back in at the `yield`, so it now bypasses the cleanup entirely. This means any failed remote media download leaves a zero-byte file in `remote_content/` (and similarly `url_cache/` for URL previews) without a corresponding database row. Most of these fail before a single byte is written, for example `NotRetryingDestination` while a remote server is backed off, which fails instantly without any network request. Media retention works from the database so it never cleans these up, and because each attempt uses a fresh `filesystem_id`, clients repeatedly requesting media (e.g. avatars of members on dead servers) keep accumulating new ones. On a small homeserver, this added up to ~6000 orphaned files (and ~36000 directories) since upgrading to v1.146.0. This PR moves the `yield` back inside the `try` (as it was before #19204) so the file is removed again on failure. This applies to both the local media store and the temporary file used when `enable_local_media_storage` is disabled. ### Dev notes To find orphaned files on an affected homeserver (zero-byte files with no matching `remote_media_cache` row): ```shell find /path/to/media_store/remote_content /path/to/media_store/url_cache -type f -size 0 ``` Any of these without a matching row in `remote_media_cache` (or `local_media_repository` for `url_cache/`) are orphans and safe to delete, along with any empty directories left behind.