diff --git a/lldb/include/lldb/Host/File.h b/lldb/include/lldb/Host/File.h --- a/lldb/include/lldb/Host/File.h +++ b/lldb/include/lldb/Host/File.h @@ -389,9 +389,7 @@ ~NativeFile() override { Close(); } - bool IsValid() const override { - return DescriptorIsValid() || StreamIsValid(); - } + bool IsValid() const override; Status Read(void *buf, size_t &num_bytes) override; Status Write(const void *buf, size_t &num_bytes) override; @@ -417,15 +415,37 @@ static bool classof(const File *file) { return file->isA(&ID); } protected: - bool DescriptorIsValid() const { + struct ValueGuard { + ValueGuard(std::mutex &m, bool b) : guard(m, std::adopt_lock), value(b) {} + std::lock_guard guard; + bool value; + operator bool() { return value; } + }; + + bool DescriptorIsValidUnlocked() const { + return File::DescriptorIsValid(m_descriptor); } - bool StreamIsValid() const { return m_stream != kInvalidStream; } - // Member variables + bool StreamIsValidUnlocked() const { return m_stream != kInvalidStream; } + + ValueGuard DescriptorIsValid() const { + m_descriptor_mutex.lock(); + return ValueGuard(m_descriptor_mutex, DescriptorIsValidUnlocked()); + } + + ValueGuard StreamIsValid() const { + m_stream_mutex.lock(); + return ValueGuard(m_stream_mutex, StreamIsValidUnlocked()); + } + int m_descriptor; bool m_own_descriptor = false; + mutable std::mutex m_descriptor_mutex; + FILE *m_stream; + mutable std::mutex m_stream_mutex; + OpenOptions m_options{}; bool m_own_stream = false; std::mutex offset_access_mutex; diff --git a/lldb/source/Host/common/File.cpp b/lldb/source/Host/common/File.cpp --- a/lldb/source/Host/common/File.cpp +++ b/lldb/source/Host/common/File.cpp @@ -219,7 +219,7 @@ size_t File::PrintfVarArg(const char *format, va_list args) { llvm::SmallString<0> s; if (VASprintf(s, format, args)) { - size_t written = s.size();; + size_t written = s.size(); Write(s.data(), written); return written; } @@ -247,15 +247,21 @@ return file_stats.st_mode & (S_IRWXU | S_IRWXG | S_IRWXO); } +bool NativeFile::IsValid() const { + std::scoped_lock lock(m_descriptor_mutex, m_stream_mutex); + return DescriptorIsValidUnlocked() || StreamIsValidUnlocked(); +} + Expected NativeFile::GetOptions() const { return m_options; } int NativeFile::GetDescriptor() const { - if (DescriptorIsValid()) + if (ValueGuard descriptor_guard = DescriptorIsValid()) { return m_descriptor; + } // Don't open the file descriptor if we don't need to, just get it from the // stream if we have one. - if (StreamIsValid()) { + if (ValueGuard stream_guard = StreamIsValid()) { #if defined(_WIN32) return _fileno(m_stream); #else @@ -272,8 +278,9 @@ } FILE *NativeFile::GetStream() { - if (!StreamIsValid()) { - if (DescriptorIsValid()) { + ValueGuard stream_guard = StreamIsValid(); + if (!stream_guard) { + if (ValueGuard descriptor_guard = DescriptorIsValid()) { auto mode = GetStreamOpenModeFromOptions(m_options); if (!mode) llvm::consumeError(mode.takeError()); @@ -282,9 +289,9 @@ // We must duplicate the file descriptor if we don't own it because when you // call fdopen, the stream will own the fd #ifdef _WIN32 - m_descriptor = ::_dup(GetDescriptor()); + m_descriptor = ::_dup(m_descriptor); #else - m_descriptor = dup(GetDescriptor()); + m_descriptor = dup(m_descriptor); #endif m_own_descriptor = true; } @@ -306,8 +313,11 @@ } Status NativeFile::Close() { + std::scoped_lock lock(m_descriptor_mutex, m_stream_mutex); + Status error; - if (StreamIsValid()) { + + if (StreamIsValidUnlocked()) { if (m_own_stream) { if (::fclose(m_stream) == EOF) error.SetErrorToErrno(); @@ -322,15 +332,17 @@ } } } - if (DescriptorIsValid() && m_own_descriptor) { + + if (DescriptorIsValidUnlocked() && m_own_descriptor) { if (::close(m_descriptor) != 0) error.SetErrorToErrno(); } - m_descriptor = kInvalidDescriptor; + m_stream = kInvalidStream; - m_options = OpenOptions(0); m_own_stream = false; + m_descriptor = kInvalidDescriptor; m_own_descriptor = false; + m_options = OpenOptions(0); m_is_interactive = eLazyBoolCalculate; m_is_real_terminal = eLazyBoolCalculate; return error; @@ -374,7 +386,7 @@ off_t NativeFile::SeekFromStart(off_t offset, Status *error_ptr) { off_t result = 0; - if (DescriptorIsValid()) { + if (ValueGuard descriptor_guard = DescriptorIsValid()) { result = ::lseek(m_descriptor, offset, SEEK_SET); if (error_ptr) { @@ -383,7 +395,10 @@ else error_ptr->Clear(); } - } else if (StreamIsValid()) { + return result; + } + + if (ValueGuard stream_guard = StreamIsValid()) { result = ::fseek(m_stream, offset, SEEK_SET); if (error_ptr) { @@ -392,15 +407,17 @@ else error_ptr->Clear(); } - } else if (error_ptr) { - error_ptr->SetErrorString("invalid file handle"); + return result; } + + if (error_ptr) + error_ptr->SetErrorString("invalid file handle"); return result; } off_t NativeFile::SeekFromCurrent(off_t offset, Status *error_ptr) { off_t result = -1; - if (DescriptorIsValid()) { + if (ValueGuard descriptor_guard = DescriptorIsValid()) { result = ::lseek(m_descriptor, offset, SEEK_CUR); if (error_ptr) { @@ -409,7 +426,10 @@ else error_ptr->Clear(); } - } else if (StreamIsValid()) { + return result; + } + + if (ValueGuard stream_guard = StreamIsValid()) { result = ::fseek(m_stream, offset, SEEK_CUR); if (error_ptr) { @@ -418,15 +438,17 @@ else error_ptr->Clear(); } - } else if (error_ptr) { - error_ptr->SetErrorString("invalid file handle"); + return result; } + + if (error_ptr) + error_ptr->SetErrorString("invalid file handle"); return result; } off_t NativeFile::SeekFromEnd(off_t offset, Status *error_ptr) { off_t result = -1; - if (DescriptorIsValid()) { + if (ValueGuard descriptor_guard = DescriptorIsValid()) { result = ::lseek(m_descriptor, offset, SEEK_END); if (error_ptr) { @@ -435,7 +457,10 @@ else error_ptr->Clear(); } - } else if (StreamIsValid()) { + return result; + } + + if (ValueGuard stream_guard = StreamIsValid()) { result = ::fseek(m_stream, offset, SEEK_END); if (error_ptr) { @@ -444,26 +469,32 @@ else error_ptr->Clear(); } - } else if (error_ptr) { - error_ptr->SetErrorString("invalid file handle"); } + + if (error_ptr) + error_ptr->SetErrorString("invalid file handle"); return result; } Status NativeFile::Flush() { Status error; - if (StreamIsValid()) { + if (ValueGuard stream_guard = StreamIsValid()) { if (llvm::sys::RetryAfterSignal(EOF, ::fflush, m_stream) == EOF) error.SetErrorToErrno(); - } else if (!DescriptorIsValid()) { - error.SetErrorString("invalid file handle"); + return error; + } + + { + ValueGuard descriptor_guard = DescriptorIsValid(); + if (!descriptor_guard) + error.SetErrorString("invalid file handle"); } return error; } Status NativeFile::Sync() { Status error; - if (DescriptorIsValid()) { + if (ValueGuard descriptor_guard = DescriptorIsValid()) { #ifdef _WIN32 int err = FlushFileBuffers((HANDLE)_get_osfhandle(m_descriptor)); if (err == 0) @@ -518,14 +549,18 @@ #endif ssize_t bytes_read = -1; - if (DescriptorIsValid()) { - bytes_read = llvm::sys::RetryAfterSignal(-1, ::read, m_descriptor, buf, num_bytes); + if (ValueGuard descriptor_guard = DescriptorIsValid()) { + bytes_read = + llvm::sys::RetryAfterSignal(-1, ::read, m_descriptor, buf, num_bytes); if (bytes_read == -1) { error.SetErrorToErrno(); num_bytes = 0; } else num_bytes = bytes_read; - } else if (StreamIsValid()) { + return error; + } + + if (ValueGuard file_lock = StreamIsValid()) { bytes_read = ::fread(buf, 1, num_bytes, m_stream); if (bytes_read == 0) { @@ -536,10 +571,11 @@ num_bytes = 0; } else num_bytes = bytes_read; - } else { - num_bytes = 0; - error.SetErrorString("invalid file handle"); + return error; } + + num_bytes = 0; + error.SetErrorString("invalid file handle"); return error; } @@ -577,7 +613,7 @@ #endif ssize_t bytes_written = -1; - if (DescriptorIsValid()) { + if (ValueGuard descriptor_guard = DescriptorIsValid()) { bytes_written = llvm::sys::RetryAfterSignal(-1, ::write, m_descriptor, buf, num_bytes); if (bytes_written == -1) { @@ -585,7 +621,10 @@ num_bytes = 0; } else num_bytes = bytes_written; - } else if (StreamIsValid()) { + return error; + } + + if (ValueGuard stream_guard = StreamIsValid()) { bytes_written = ::fwrite(buf, 1, num_bytes, m_stream); if (bytes_written == 0) { @@ -596,12 +635,11 @@ num_bytes = 0; } else num_bytes = bytes_written; - - } else { - num_bytes = 0; - error.SetErrorString("invalid file handle"); + return error; } + num_bytes = 0; + error.SetErrorString("invalid file handle"); return error; }