From 0ac2376e077ce81088923af12430a80abf6f6973 Mon Sep 17 00:00:00 2001 From: Carey Metcalfe Date: Sun, 3 Feb 2019 19:02:56 -0500 Subject: [PATCH] Fix issues with hanging transcoding processes When a connection that is consuming a generated response is closed, Flask closes the generator making it raise the special `GeneratorExit` exception when the program tries to yield from it again. Because the `transcode` function was called (returning a generator) before being passed into `set_generated`, the exception was being handled in the wrong order. By passing the `transcode` function to `set_generated` and letting `set_transcode` call it to return a generator while generating the response for the client, the exception properly bubbles up through `transcode` into `set_generated`. This allows both of them to handle it properly by stopping the subproceses and not caching the incomplete response data respectively. --- supysonic/api/media.py | 2 +- supysonic/cache.py | 12 +++++++----- tests/base/test_cache.py | 10 +++++----- 3 files changed, 13 insertions(+), 11 deletions(-) diff --git a/supysonic/api/media.py b/supysonic/api/media.py index c588d2e..7d72643 100644 --- a/supysonic/api/media.py +++ b/supysonic/api/media.py @@ -127,7 +127,7 @@ def stream_media(): if dec_proc != None: dec_proc.wait() proc.wait() - resp_content = cache.set_generated(cache_key, transcode()) + resp_content = cache.set_generated(cache_key, transcode) logger.info('Transcoding track {0.id} for user {1.id}. Source: {2} at {0.bitrate}kbps. Dest: {3} at {4}kbps'.format(res, request.user, src_suffix, dst_suffix, dst_bitrate)) response = Response(resp_content, mimetype=dst_mimetype) diff --git a/supysonic/cache.py b/supysonic/cache.py index 125f1a8..b1b7b35 100644 --- a/supysonic/cache.py +++ b/supysonic/cache.py @@ -161,17 +161,19 @@ class Cache(object): f.write(value) return self._filepath(key) - def set_generated(self, key, gen): - """Pass the generated values through and set the end result in the cache + def set_generated(self, key, gen_function): + """Pass the values yielded from the generator function through and set + the end result in the cache. - The contents will be set into the cache when the generator completes. + The contents will be set into the cache only if and when the generator + completes. Ex: - >>> for x in cache.set_generated(key, some_generator()): + >>> for x in cache.set_generated(key, generator_function): ... print(x) """ with self.set_fileobj(key) as f: - for data in gen: + for data in gen_function(): f.write(data) yield data diff --git a/tests/base/test_cache.py b/tests/base/test_cache.py index d5c2a30..a134b10 100644 --- a/tests/base/test_cache.py +++ b/tests/base/test_cache.py @@ -76,7 +76,7 @@ class CacheTestCase(unittest.TestCase): yield b t = [] - for x in cache.set_generated("key", gen()): + for x in cache.set_generated("key", gen): t.append(x) self.assertEqual(cache.size, 0) self.assertFalse(cache.has("key")) @@ -103,7 +103,7 @@ class CacheTestCase(unittest.TestCase): self.assertEqual(cache.get_value("key"), val) with cache.get_fileobj("key") as f: - self.assertEquals(f.read(), val) + self.assertEqual(f.read(), val) with open(cache.get("key"), 'rb') as f: self.assertEqual(f.read(), val) @@ -162,7 +162,7 @@ class CacheTestCase(unittest.TestCase): yield b with self.assertRaises(TypeError): - for x in cache.set_generated("key", gen()): + for x in cache.set_generated("key", gen): pass # Make sure no partial files are left after the error @@ -174,8 +174,8 @@ class CacheTestCase(unittest.TestCase): for b in [b'0', b'12', b'345', b'6789']: yield b - g1 = cache.set_generated("key", gen()) - g2 = cache.set_generated("key", gen()) + g1 = cache.set_generated("key", gen) + g2 = cache.set_generated("key", gen) next(g1) files = os.listdir(self.__dir)