More immediate resource closures

src/calibre/db/backend.py

@@ -1570,25 +1570,27 @@ class DB:
             return fmt_path
         if not fmt:
             return
-        candidates = ()
-        with suppress(OSError):
-            candidates = os.scandir(path)
         q = fmt.lower()
-        for x in candidates:
+        try:
-            if x.name.endswith(q) and x.is_file():
+            candidates = os.scandir(path)
-                if not do_file_rename:
+        except OSError:
-                    return x.path
+            return
-                x = x.path
+        with candidates:
-                with suppress(OSError):
+            for x in candidates:
-                    atomic_rename(x, fmt_path)
+                if x.name.endswith(q) and x.is_file():
+                    if not do_file_rename:
+                        return x.path
+                    x = x.path
+                    with suppress(OSError):
+                        atomic_rename(x, fmt_path)
+                        return fmt_path
+                    try:
+                        shutil.move(x, fmt_path)
+                    except (shutil.SameFileError, OSError):
+                        # some other process synced in the file since the last
+                        # os.path.exists()
+                        return x
                     return fmt_path
-                try:
-                    shutil.move(x, fmt_path)
-                except (shutil.SameFileError, OSError):
-                    # some other process synced in the file since the last
-                    # os.path.exists()
-                    return x
-                return fmt_path
 
     def cover_abspath(self, book_id, path):
         path = os.path.join(self.library_path, path)

src/calibre/db/cache.py

@@ -1130,6 +1130,7 @@ class Cache:
         if as_file:
             ret = SpooledTemporaryFile(SPOOL_SIZE)
             if not self.copy_cover_to(book_id, ret):
+                ret.close()
                 return
             ret.seek(0)
         elif as_path:
@@ -1379,6 +1380,7 @@ class Cache:
             try:
                 self.copy_format_to(book_id, fmt, ret)
             except NoSuchFormat:
+                ret.close()
                 return None
             ret.seek(0)
             # Various bits of code try to use the name as the default
@@ -1953,6 +1955,8 @@ class Cache:
                 name = None
 
             if name and not replace:
+                if needs_close:
+                    stream_or_path.close()
                 return False
 
             if hasattr(stream_or_path, 'read'):
@@ -1961,7 +1965,6 @@ class Cache:
                 stream = open(stream_or_path, 'rb')
                 needs_close = True
             try:
-                stream = stream_or_path if hasattr(stream_or_path, 'read') else open(stream_or_path, 'rb')
                 size, fname = self._do_add_format(book_id, fmt, stream, name)
             finally:
                 if needs_close:

src/calibre/db/tests/add_remove.py

@@ -370,6 +370,10 @@ class AddRemoveTest(BaseTest):
         src_db = self.init_cache()
         dest_db = self.init_cache(self.cloned_library)
 
+        def read(x, mode='r'):
+            with open(x, mode) as f:
+                return f.read()
+
         def a(**kw):
             ts = utcnow()
             kw['timestamp'] = utcnow().isoformat()
@@ -401,8 +405,8 @@ class AddRemoveTest(BaseTest):
 
         def assert_has_extra_files(book_id):
             bookdir = os.path.dirname(dest_db.format_abspath(book_id, '__COVER_INTERNAL__'))
-            self.assertEqual('exf', open(os.path.join(bookdir, 'exf')).read())
+            self.assertEqual('exf', read(os.path.join(bookdir, 'exf')))
-            self.assertEqual('recurse', open(os.path.join(bookdir, 'sub', 'recurse')).read())
+            self.assertEqual('recurse', read(os.path.join(bookdir, 'sub', 'recurse')))
 
         def assert_does_not_have_extra_files(book_id):
             bookdir = os.path.dirname(dest_db.format_abspath(book_id, '__COVER_INTERNAL__'))

src/calibre/db/tests/reading.py

@@ -270,7 +270,11 @@ class ReadingTest(BaseTest):
         for book_id, cdata in iteritems(covers):
             self.assertEqual(cdata, cache.cover(book_id), 'Reading of cover failed')
             f = cache.cover(book_id, as_file=True)
-            self.assertEqual(cdata, f.read() if f else f, 'Reading of cover as file failed')
+            try:
+                self.assertEqual(cdata, f.read() if f else f, 'Reading of cover as file failed')
+            finally:
+                if f:
+                    f.close()
             if cdata:
                 with open(cache.cover(book_id, as_path=True), 'rb') as f:
                     self.assertEqual(cdata, f.read(), 'Reading of cover as path failed')
@@ -441,9 +445,9 @@ class ReadingTest(BaseTest):
                 old = formats[book_id][fmt]
                 self.assertEqual(old, cache.format(book_id, fmt),
                                  'Old and new format disagree')
-                f = cache.format(book_id, fmt, as_file=True)
+                with cache.format(book_id, fmt, as_file=True) as f:
-                self.assertEqual(old, f.read(),
+                    self.assertEqual(old, f.read(),
-                                 'Failed to read format as file')
+                                    'Failed to read format as file')
                 with open(cache.format(book_id, fmt, as_path=True,
                                        preserve_filename=True), 'rb') as f:
                     self.assertEqual(old, f.read(),