From d9cc81766d6a917a50fac6804ea8267197980f91 Mon Sep 17 00:00:00 2001 From: Kay Kurokawa Date: Fri, 6 Oct 2017 17:05:10 -0400 Subject: [PATCH] use DeferredLock instead of threading.Lock() as it can cause deadlocks --- lbrynet/blob/blob_file.py | 29 +++++++++++++----------- lbrynet/tests/unit/core/test_HashBlob.py | 4 ++-- 2 files changed, 18 insertions(+), 15 deletions(-) diff --git a/lbrynet/blob/blob_file.py b/lbrynet/blob/blob_file.py index 993a77400..2abc10f3a 100644 --- a/lbrynet/blob/blob_file.py +++ b/lbrynet/blob/blob_file.py @@ -1,6 +1,5 @@ import logging import os -import threading from twisted.internet import defer, threads from twisted.protocols.basic import FileSender from twisted.web.client import FileBodyProducer @@ -40,7 +39,7 @@ class BlobFile(object): self.readers = 0 self.blob_dir = blob_dir self.file_path = os.path.join(blob_dir, self.blob_hash) - self.setting_verified_blob_lock = threading.Lock() + self.blob_write_lock = defer.DeferredLock() self.moved_verified_blob = False if os.path.isfile(self.file_path): self.set_length(os.path.getsize(self.file_path)) @@ -201,7 +200,7 @@ class BlobFile(object): if err is None: if writer.len_so_far == self.length and writer.blob_hash == self.blob_hash: if self._verified is False: - d = self._save_verified_blob(writer) + d = self.save_verified_blob(writer) d.addCallbacks(lambda _: fire_finished_deferred(), errback_finished_deferred) d.addCallback(lambda _: cancel_other_downloads()) else: @@ -219,15 +218,19 @@ class BlobFile(object): d.addBoth(lambda _: writer.close_handle()) return d + def save_verified_blob(self, writer): + # we cannot have multiple _save_verified_blob interrupting + # each other, can happen since startProducing is a deferred + return self.blob_write_lock.run(self._save_verified_blob, writer) + @defer.inlineCallbacks def _save_verified_blob(self, writer): - with self.setting_verified_blob_lock: - if self.moved_verified_blob is False: - writer.write_handle.seek(0) - out_path = os.path.join(self.blob_dir, self.blob_hash) - producer = FileBodyProducer(writer.write_handle) - yield producer.startProducing(open(out_path, 'wb')) - self.moved_verified_blob = True - defer.returnValue(True) - else: - raise DownloadCanceledError() + if self.moved_verified_blob is False: + writer.write_handle.seek(0) + out_path = os.path.join(self.blob_dir, self.blob_hash) + producer = FileBodyProducer(writer.write_handle) + yield producer.startProducing(open(out_path, 'wb')) + self.moved_verified_blob = True + defer.returnValue(True) + else: + raise DownloadCanceledError() diff --git a/lbrynet/tests/unit/core/test_HashBlob.py b/lbrynet/tests/unit/core/test_HashBlob.py index 888887bb8..66cc1758e 100644 --- a/lbrynet/tests/unit/core/test_HashBlob.py +++ b/lbrynet/tests/unit/core/test_HashBlob.py @@ -150,7 +150,7 @@ class BlobFileTest(unittest.TestCase): writer_1, finished_d_1 = blob_file.open_for_writing(peer=1) writer_2, finished_d_2 = blob_file.open_for_writing(peer=2) - blob_file._save_verified_blob(writer_1) + blob_file.save_verified_blob(writer_1) # second write should fail to save - yield self.assertFailure(blob_file._save_verified_blob(writer_2), DownloadCanceledError) + yield self.assertFailure(blob_file.save_verified_blob(writer_2), DownloadCanceledError)