From 51a7ff85c2af34f97e845710998b18e691092565 Mon Sep 17 00:00:00 2001 From: Thomas Inskip Date: Fri, 13 Nov 2015 18:43:25 -0800 Subject: [PATCH] Fixed a miniscule odds race condition found by tsan (Thread sanitizer) in ThreadedIofile. Added loop to handle partial writes in ThreadedIoFile. Change-Id: Ib62855ab849ffbfd00afc5b226dd81d4cd38ff51 --- packager/media/file/threaded_io_file.cc | 35 +++++++++++++++---------- packager/media/file/threaded_io_file.h | 5 ++-- 2 files changed, 24 insertions(+), 16 deletions(-) diff --git a/packager/media/file/threaded_io_file.cc b/packager/media/file/threaded_io_file.cc index c14ee8169c..98d3ce9fa7 100644 --- a/packager/media/file/threaded_io_file.cc +++ b/packager/media/file/threaded_io_file.cc @@ -14,6 +14,9 @@ namespace edash_packager { namespace media { +using base::subtle::NoBarrier_Load; +using base::subtle::NoBarrier_Store; + ThreadedIoFile::ThreadedIoFile(scoped_ptr internal_file, Mode mode, uint64_t io_cache_size, @@ -70,12 +73,12 @@ int64_t ThreadedIoFile::Read(void* buffer, uint64_t length) { DCHECK(thread_); DCHECK_EQ(kInputMode, mode_); - if (internal_file_error_) - return internal_file_error_; - - if (eof_ && !cache_.BytesCached()) + if (NoBarrier_Load(&eof_) && !cache_.BytesCached()) return 0; + if (NoBarrier_Load(&internal_file_error_)) + return NoBarrier_Load(&internal_file_error_); + return cache_.Read(buffer, length); } @@ -84,8 +87,8 @@ int64_t ThreadedIoFile::Write(const void* buffer, uint64_t length) { DCHECK(thread_); DCHECK_EQ(kOutputMode, mode_); - if (internal_file_error_) - return internal_file_error_; + if (NoBarrier_Load(&internal_file_error_)) + return NoBarrier_Load(&internal_file_error_); size_ += length; return cache_.Write(buffer, length); @@ -118,8 +121,8 @@ void ThreadedIoFile::RunInInputMode() { int64_t read_result = internal_file_->Read(&io_buffer_[0], io_buffer_.size()); if (read_result <= 0) { - eof_ = read_result == 0; - internal_file_error_ = read_result; + NoBarrier_Store(&eof_, read_result == 0); + NoBarrier_Store(&internal_file_error_, read_result); cache_.Close(); return; } @@ -153,13 +156,17 @@ void ThreadedIoFile::RunInOutputMode() { return; } } else { - int64_t write_result = internal_file_->Write(&io_buffer_[0], write_bytes); - if (write_result < 0) { - internal_file_error_ = write_result; - cache_.Close(); - return; + uint64_t bytes_written(0); + while (bytes_written < write_bytes) { + int64_t write_result = internal_file_->Write( + &io_buffer_[bytes_written], write_bytes - bytes_written); + if (write_result < 0) { + NoBarrier_Store(&internal_file_error_, write_result); + cache_.Close(); + return; + } + bytes_written += write_result; } - CHECK_EQ(write_result, static_cast(write_bytes)); } } } diff --git a/packager/media/file/threaded_io_file.h b/packager/media/file/threaded_io_file.h index 70e37f3f71..4d961c2461 100644 --- a/packager/media/file/threaded_io_file.h +++ b/packager/media/file/threaded_io_file.h @@ -7,6 +7,7 @@ #ifndef PACKAGER_FILE_THREADED_IO_FILE_H_ #define PACKAGER_FILE_THREADED_IO_FILE_H_ +#include "packager/base/atomicops.h" #include "packager/base/memory/scoped_ptr.h" #include "packager/base/synchronization/lock.h" #include "packager/base/synchronization/waitable_event.h" @@ -58,10 +59,10 @@ class ThreadedIoFile : public File { std::vector io_buffer_; scoped_ptr thread_; uint64_t size_; - bool eof_; + base::subtle::Atomic32 eof_; bool flushing_; base::WaitableEvent flush_complete_event_; - int64_t internal_file_error_; + base::subtle::Atomic32 internal_file_error_; DISALLOW_COPY_AND_ASSIGN(ThreadedIoFile); };