From 802ca12d0551ff761e01d9af8348df1dc96fc372 Mon Sep 17 00:00:00 2001
From: Erik Johnston <erik@matrix.org>
Date: Thu, 12 Oct 2017 17:37:21 +0100
Subject: Don't close file prematurely

---
 synapse/rest/media/v1/media_repository.py     | 22 ++++++++++++++++------
 synapse/rest/media/v1/preview_url_resource.py |  4 ++--
 2 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/synapse/rest/media/v1/media_repository.py b/synapse/rest/media/v1/media_repository.py
index 398e973ca9..63ed1c4268 100644
--- a/synapse/rest/media/v1/media_repository.py
+++ b/synapse/rest/media/v1/media_repository.py
@@ -97,16 +97,20 @@ class MediaRepository(object):
             os.makedirs(dirname)
 
     @staticmethod
-    def write_file_synchronously(source, fname):
+    def _write_file_synchronously(source, fname):
         source.seek(0)  # Ensure we read from the start of the file
         with open(fname, "wb") as f:
             shutil.copyfileobj(source, f)
 
+        source.close()
+
     @defer.inlineCallbacks
     def write_to_file(self, source, path):
         """Write `source` to the on disk media store, and also the backup store
         if configured.
 
+        Will close source once finished.
+
         Args:
             source: A file like object that should be written
             path: Relative path to write file to
@@ -120,7 +124,7 @@ class MediaRepository(object):
         # Write to the main repository
         yield preserve_context_over_fn(
             threads.deferToThread,
-            self.write_file_synchronously, source, fname,
+            self._write_file_synchronously, source, fname,
         )
 
         # Write to backup repository
@@ -130,6 +134,10 @@ class MediaRepository(object):
 
     @defer.inlineCallbacks
     def copy_to_backup(self, source, path):
+        """Copy file like object source to the backup media store, if configured.
+
+        Will close source after its done.
+        """
         if self.backup_base_path:
             backup_fname = os.path.join(self.backup_base_path, path)
             self._makedirs(backup_fname)
@@ -139,12 +147,14 @@ class MediaRepository(object):
             if self.synchronous_backup_media_store:
                 yield preserve_context_over_fn(
                     threads.deferToThread,
-                    self.write_file_synchronously, source, backup_fname,
+                    self._write_file_synchronously, source, backup_fname,
                 )
             else:
                 preserve_fn(threads.deferToThread)(
-                    self.write_file_synchronously, source, backup_fname,
+                    self._write_file_synchronously, source, backup_fname,
                 )
+        else:
+            source.close()
 
     @defer.inlineCallbacks
     def create_content(self, media_type, upload_name, content, content_length,
@@ -248,8 +258,8 @@ class MediaRepository(object):
                                      server_name, media_id)
                     raise SynapseError(502, "Failed to fetch remote media")
 
-            with open(fname) as f:
-                yield self.copy_to_backup(f, fpath)
+             # Will close the file after its done
+            yield self.copy_to_backup(open(fname), fpath)
 
             media_type = headers["Content-Type"][0]
             time_now_ms = self.clock.time_msec()
diff --git a/synapse/rest/media/v1/preview_url_resource.py b/synapse/rest/media/v1/preview_url_resource.py
index f82b8fbc51..a3288c9cc6 100644
--- a/synapse/rest/media/v1/preview_url_resource.py
+++ b/synapse/rest/media/v1/preview_url_resource.py
@@ -275,8 +275,8 @@ class PreviewUrlResource(Resource):
                 )
                 # FIXME: pass through 404s and other error messages nicely
 
-            with open(fname) as f:
-                yield self.media_repo.copy_to_backup(f, fpath)
+            # Will close the file after its done
+            yield self.media_repo.copy_to_backup(open(fname), fpath)
 
             media_type = headers["Content-Type"][0]
             time_now_ms = self.clock.time_msec()
-- 
cgit 1.4.1