From 9ec3d4015b8a315379d5d516ca82ab9ff803a081 Mon Sep 17 00:00:00 2001 From: Per Date: Thu, 22 Aug 2024 16:48:43 +0200 Subject: [PATCH] Handle soft signal interrupts during file processing (#1597) * Handle soft signal interrupts during file processing These signals are sometimes randomly encountered on networked filesystems and should be handled by resuming the operation where they left off. --- framework/decode/dx12_dump_resources.cpp | 6 +- framework/decode/file_processor.cpp | 9 ++- framework/decode/file_transformer.cpp | 18 ++++-- framework/decode/json_writer.cpp | 4 +- framework/decode/preload_file_processor.cpp | 2 +- framework/decode/vulkan_cpp_util_datapack.cpp | 3 +- .../decode/vulkan_replay_consumer_base.cpp | 2 +- .../vulkan_replay_dump_resources_json.cpp | 8 +-- framework/graphics/fps_info.cpp | 7 +- framework/util/buffer_writer.cpp | 8 +-- framework/util/file_output_stream.cpp | 8 +-- framework/util/file_output_stream.h | 4 +- framework/util/image_writer.cpp | 28 ++++---- framework/util/json_util.cpp | 11 +--- framework/util/logging.cpp | 2 +- framework/util/memory_output_stream.cpp | 4 +- framework/util/memory_output_stream.h | 2 +- framework/util/output_stream.h | 2 +- framework/util/platform.h | 64 +++++++++++++------ tools/extract/main.cpp | 3 +- 20 files changed, 109 insertions(+), 86 deletions(-) diff --git a/framework/decode/dx12_dump_resources.cpp b/framework/decode/dx12_dump_resources.cpp index efe8343348..419b3101da 100644 --- a/framework/decode/dx12_dump_resources.cpp +++ b/framework/decode/dx12_dump_resources.cpp @@ -1731,7 +1731,7 @@ void DefaultDx12DumpResourcesDelegate::WriteBlockEnd() // Dominates profiling (2/2): const std::string block = json_data_.dump(json_options_.format == util::JsonFormat::JSONL ? -1 : util::kJsonIndentWidth); - util::platform::FileWriteNoLock(block.data(), sizeof(std::string::value_type), block.length(), json_file_handle_); + util::platform::FileWriteNoLock(block.data(), block.length() * sizeof(std::string::value_type), json_file_handle_); util::platform::FileFlush(json_file_handle_); /// @todo Implement a FileFlushNoLock() for all platforms. } @@ -1743,9 +1743,9 @@ bool DefaultDx12DumpResourcesDelegate::WriteBinaryFile(const std::string& FILE* file_output = nullptr; if (util::platform::FileOpen(&file_output, filename.c_str(), "wb") == 0) { - util::platform::FileWrite(data.data() + offset, size, 1, file_output); + bool success = util::platform::FileWrite(data.data() + offset, size, file_output); util::platform::FileClose(file_output); - return true; + return success; } return false; } diff --git a/framework/decode/file_processor.cpp b/framework/decode/file_processor.cpp index aa473f7c16..304fe3bc2f 100644 --- a/framework/decode/file_processor.cpp +++ b/framework/decode/file_processor.cpp @@ -467,9 +467,12 @@ bool FileProcessor::ReadCompressedParameterBuffer(size_t compressed_buffer_size bool FileProcessor::ReadBytes(void* buffer, size_t buffer_size) { - size_t bytes_read = util::platform::FileRead(buffer, 1, buffer_size, file_descriptor_); - bytes_read_ += bytes_read; - return (bytes_read == buffer_size); + if (util::platform::FileRead(buffer, buffer_size, file_descriptor_)) + { + bytes_read_ += buffer_size; + return true; + } + return false; } bool FileProcessor::SkipBytes(size_t skip_size) diff --git a/framework/decode/file_transformer.cpp b/framework/decode/file_transformer.cpp index e15ef85543..4d4aefd372 100644 --- a/framework/decode/file_transformer.cpp +++ b/framework/decode/file_transformer.cpp @@ -352,16 +352,22 @@ bool FileTransformer::ReadCompressedParameterBuffer(size_t compressed_buffer_si bool FileTransformer::ReadBytes(void* buffer, size_t buffer_size) { - size_t bytes_read = util::platform::FileRead(buffer, 1, buffer_size, input_file_); - bytes_read_ += bytes_read; - return (bytes_read == buffer_size); + if (util::platform::FileRead(buffer, buffer_size, input_file_)) + { + bytes_read_ += buffer_size; + return true; + } + return false; } bool FileTransformer::WriteBytes(const void* buffer, size_t buffer_size) { - size_t bytes_written = util::platform::FileWrite(buffer, 1, buffer_size, output_file_); - bytes_written_ += bytes_written; - return (bytes_written == buffer_size); + if (util::platform::FileWrite(buffer, buffer_size, output_file_)) + { + bytes_written_ += buffer_size; + return true; + } + return false; } bool FileTransformer::SkipBytes(uint64_t skip_size) diff --git a/framework/decode/json_writer.cpp b/framework/decode/json_writer.cpp index 1bfb44296f..5dfc0ade13 100644 --- a/framework/decode/json_writer.cpp +++ b/framework/decode/json_writer.cpp @@ -206,9 +206,9 @@ bool JsonWriter::WriteBinaryFile(const std::string& filename, uint64_t data_size FILE* file_output = nullptr; if (util::platform::FileOpen(&file_output, filename.c_str(), "wb") == 0) { - util::platform::FileWrite(data, static_cast(data_size), 1, file_output); + bool success = util::platform::FileWrite(data, static_cast(data_size), file_output); util::platform::FileClose(file_output); - return true; + return success; } return false; } diff --git a/framework/decode/preload_file_processor.cpp b/framework/decode/preload_file_processor.cpp index 5f3a8b7514..7c9d86f6ae 100644 --- a/framework/decode/preload_file_processor.cpp +++ b/framework/decode/preload_file_processor.cpp @@ -335,7 +335,7 @@ bool PreloadFileProcessor::ReadBytes(void* buffer, size_t buffer_size) } else { - bytes_read = util::platform::FileRead(buffer, 1, buffer_size, file_descriptor_); + bytes_read = util::platform::FileRead(buffer, buffer_size, file_descriptor_); bytes_read_ += bytes_read; } return bytes_read == buffer_size; diff --git a/framework/decode/vulkan_cpp_util_datapack.cpp b/framework/decode/vulkan_cpp_util_datapack.cpp index 9a25e4ca4f..d2dbd16d49 100644 --- a/framework/decode/vulkan_cpp_util_datapack.cpp +++ b/framework/decode/vulkan_cpp_util_datapack.cpp @@ -86,8 +86,7 @@ void DataFilePacker::WriteContentsToFile(const std::string& file_path, util::platform::FileSeek(fp, fileOffset, util::platform::FileSeekCurrent); - size_t written_size = util::platform::FileWrite(data, sizeof(uint8_t), size, fp); - if (written_size != size) + if (!util::platform::FileWrite(data, size, fp)) { fprintf(stderr, "Error while saving data into %s\n", file_path.c_str()); } diff --git a/framework/decode/vulkan_replay_consumer_base.cpp b/framework/decode/vulkan_replay_consumer_base.cpp index bf0eec803c..e7335b0aaf 100644 --- a/framework/decode/vulkan_replay_consumer_base.cpp +++ b/framework/decode/vulkan_replay_consumer_base.cpp @@ -5641,7 +5641,7 @@ VkResult VulkanReplayConsumerBase::OverrideCreateShaderModule( size_t file_size = static_cast(util::platform::FileTell(fp)); file_code = std::make_unique(file_size); util::platform::FileSeek(fp, 0L, util::platform::FileSeekSet); - util::platform::FileRead(file_code.get(), sizeof(char), file_size, fp); + util::platform::FileRead(file_code.get(), file_size, fp); override_info.pCode = (uint32_t*)file_code.get(); override_info.codeSize = file_size; GFXRECON_LOG_INFO("Replacement shader found: %s", file_path.c_str()); diff --git a/framework/decode/vulkan_replay_dump_resources_json.cpp b/framework/decode/vulkan_replay_dump_resources_json.cpp index fcecd76eca..948e5ed166 100644 --- a/framework/decode/vulkan_replay_dump_resources_json.cpp +++ b/framework/decode/vulkan_replay_dump_resources_json.cpp @@ -63,7 +63,7 @@ bool VulkanReplayDumpResourcesJson::InitializeFile(const std::string& filename) return false; } - util::platform::FileWrite("[\n", 2, 1, file_); + util::platform::FileWrite("[\n", 2, file_); BlockStart(); json_data_["header"] = header_; @@ -110,7 +110,7 @@ void VulkanReplayDumpResourcesJson::Close() { if (file_ != nullptr) { - util::platform::FileWrite("]", 1, 1, file_); + util::platform::FileWrite("]", 1, file_); gfxrecon::util::platform::FileClose(file_); file_ = nullptr; } @@ -130,13 +130,13 @@ void VulkanReplayDumpResourcesJson::BlockEnd() if (!first_block_) { - util::platform::FileWrite(",\n", 2, 1, file_); + util::platform::FileWrite(",\n", 2, file_); } first_block_ = false; const std::string block = json_data_.dump(util::kJsonIndentWidth); - util::platform::FileWrite(block.c_str(), block.size(), 1, file_); + util::platform::FileWrite(block.c_str(), block.size(), file_); } nlohmann::ordered_json& VulkanReplayDumpResourcesJson::InsertSubEntry(const std::string& entry_name) diff --git a/framework/graphics/fps_info.cpp b/framework/graphics/fps_info.cpp index eb52010309..db20444a16 100644 --- a/framework/graphics/fps_info.cpp +++ b/framework/graphics/fps_info.cpp @@ -147,12 +147,8 @@ void FpsInfo::EndFrame(uint64_t frame) { const std::string json_string = file_content.dump(util::kJsonIndentWidth); - const size_t size_written = - util::platform::FileWrite(json_string.data(), 1, json_string.size(), file_pointer); - util::platform::FileClose(file_pointer); - // It either writes a fully valid file, or it doesn't write anything ! - if (size_written != json_string.size()) + if (!util::platform::FileWrite(json_string.data(), json_string.size(), file_pointer)) { GFXRECON_LOG_ERROR("Failed to write to measurements file '%s'.", measurement_file_name_.c_str()); @@ -166,6 +162,7 @@ void FpsInfo::EndFrame(uint64_t frame) remove_result); } } + util::platform::FileClose(file_pointer); } else { diff --git a/framework/util/buffer_writer.cpp b/framework/util/buffer_writer.cpp index 88566009ce..344e0db6e9 100644 --- a/framework/util/buffer_writer.cpp +++ b/framework/util/buffer_writer.cpp @@ -44,15 +44,11 @@ bool WriteBuffer(const std::string& filename, const void* data, size_t size) return false; } - size_t ret = util::platform::FileWrite(data, size, 1, file); - if (ret != 1) - { - return false; - } + bool success = util::platform::FileWrite(data, size, file); util::platform::FileClose(file); - return true; + return success; } GFXRECON_END_NAMESPACE(gfxrecon) diff --git a/framework/util/file_output_stream.cpp b/framework/util/file_output_stream.cpp index c19fffae2d..608301dbe4 100644 --- a/framework/util/file_output_stream.cpp +++ b/framework/util/file_output_stream.cpp @@ -69,14 +69,14 @@ void FileOutputStream::Reset(FILE* file) file_ = file; } -size_t FileOutputStream::Write(const void* data, size_t len) +bool FileOutputStream::Write(const void* data, size_t len) { - return platform::FileWrite(data, 1, len, file_); + return platform::FileWrite(data, len, file_); } -size_t FileNoLockOutputStream::Write(const void* data, size_t len) +bool FileNoLockOutputStream::Write(const void* data, size_t len) { - return platform::FileWriteNoLock(data, 1, len, file_); + return platform::FileWriteNoLock(data, len, file_); } GFXRECON_END_NAMESPACE(util) diff --git a/framework/util/file_output_stream.h b/framework/util/file_output_stream.h index 3c46caedb2..e8c65f1792 100644 --- a/framework/util/file_output_stream.h +++ b/framework/util/file_output_stream.h @@ -54,7 +54,7 @@ class FileOutputStream : public OutputStream virtual bool IsValid() override { return (file_ != nullptr); } - virtual size_t Write(const void* data, size_t len) override; + virtual bool Write(const void* data, size_t len) override; virtual void Flush() override { platform::FileFlush(file_); } @@ -75,7 +75,7 @@ class FileNoLockOutputStream : public FileOutputStream {} FileNoLockOutputStream(FILE* file, bool owned = false) : FileOutputStream(file, owned) {} - virtual size_t Write(const void* data, size_t len) override; + virtual bool Write(const void* data, size_t len) override; }; GFXRECON_END_NAMESPACE(util) diff --git a/framework/util/image_writer.cpp b/framework/util/image_writer.cpp index 4ec2e2f6e2..01020adc0e 100644 --- a/framework/util/image_writer.cpp +++ b/framework/util/image_writer.cpp @@ -215,9 +215,9 @@ static float Ufloat10ToFloat(uint16_t val) } } -#define CheckFwriteRetVal(_val_, _expected_, _file_) \ +#define CheckFwriteRetVal(_val_, _file_) \ { \ - if (_val_ != _expected_) \ + if (!_val_) \ { \ GFXRECON_LOG_ERROR("%s() (%u): fwrite failed (%s)", __func__, __LINE__, strerror(errno)); \ util::platform::FileClose(_file_); \ @@ -711,11 +711,11 @@ bool WriteBmpImage(const std::string& filename, info_header.clr_used = 0; info_header.clr_important = 0; - size_t ret = util::platform::FileWrite(&file_header, sizeof(file_header), 1, file); - CheckFwriteRetVal(ret, 1, file); + bool ret = util::platform::FileWrite(&file_header, sizeof(file_header), file); + CheckFwriteRetVal(ret, file); - ret = util::platform::FileWrite(&info_header, sizeof(info_header), 1, file); - CheckFwriteRetVal(ret, 1, file); + ret = util::platform::FileWrite(&info_header, sizeof(info_header), file); + CheckFwriteRetVal(ret, file); // Y needs to be inverted when writing the bitmap data. auto height_1 = height - 1; @@ -725,8 +725,8 @@ bool WriteBmpImage(const std::string& filename, for (uint32_t y = 0; y < height; ++y) { const uint8_t* bytes = reinterpret_cast(data); - ret = util::platform::FileWrite(&bytes[(height_1 - y) * data_pitch], 1, data_pitch, file); - CheckFwriteRetVal(ret, bmp_pitch, file); + ret = util::platform::FileWrite(&bytes[(height_1 - y) * data_pitch], data_pitch, file); + CheckFwriteRetVal(ret, file); } } else @@ -735,8 +735,8 @@ bool WriteBmpImage(const std::string& filename, ConvertIntoTemporaryBuffer(width, height, data, data_pitch, format, false, write_alpha); for (uint32_t y = 0; y < height; ++y) { - ret = util::platform::FileWrite(&bytes[(height_1 - y) * bmp_pitch], 1, bmp_pitch, file); - CheckFwriteRetVal(ret, bmp_pitch, file); + ret = util::platform::FileWrite(&bytes[(height_1 - y) * bmp_pitch], bmp_pitch, file); + CheckFwriteRetVal(ret, file); } } @@ -845,12 +845,12 @@ bool WriteAstcImage(const std::string& filename, if (!result && file != nullptr) { // Write the header - int ret = util::platform::FileWrite(&header, sizeof(header), 1, file); - CheckFwriteRetVal(ret, 1, file); + bool ret = util::platform::FileWrite(&header, sizeof(header), file); + CheckFwriteRetVal(ret, file); // Write the binary payload - ret = util::platform::FileWrite(data, size, 1, file); - CheckFwriteRetVal(ret, 1, file); + ret = util::platform::FileWrite(data, size, file); + CheckFwriteRetVal(ret, file); if (!ferror(file)) { diff --git a/framework/util/json_util.cpp b/framework/util/json_util.cpp index 969bc4a31b..e2fda88abd 100644 --- a/framework/util/json_util.cpp +++ b/framework/util/json_util.cpp @@ -352,15 +352,10 @@ static bool WriteBinaryFile(const std::string& filename, uint64_t data_size, con bool written_all = false; if (util::platform::FileOpen(&file_output, filename.c_str(), "wb") == 0) { - const uint64_t written = util::platform::FileWrite(data, 1, static_cast(data_size), file_output); - if (written >= data_size) + written_all = util::platform::FileWrite(data, static_cast(data_size), file_output); + if (!written_all) { - written_all = true; - } - else - { - GFXRECON_LOG_ERROR( - "Only wrote %" PRIu64 " bytes of %" PRIu64 " data to file %s.", written, data_size, filename.c_str()); + GFXRECON_LOG_ERROR("Failed to write %" PRIu64 " bytes to file %s.", data_size, filename.c_str()); } util::platform::FileClose(file_output); } diff --git a/framework/util/logging.cpp b/framework/util/logging.cpp index bd3c4e4cd7..2411dd8e44 100644 --- a/framework/util/logging.cpp +++ b/framework/util/logging.cpp @@ -293,7 +293,7 @@ void Log::LogMessage( // Write the newline since we want to separate each log-line but don't // want the messages themselves to have to add it. output_message = "\n"; - platform::FileWrite(output_message.c_str(), 1, 1, log_file_ptr); + platform::FileWrite(output_message.c_str(), 1, log_file_ptr); if (settings_.flush_after_write || settings_.leave_file_open) { diff --git a/framework/util/memory_output_stream.cpp b/framework/util/memory_output_stream.cpp index 966ffa4bfd..855a5c1412 100644 --- a/framework/util/memory_output_stream.cpp +++ b/framework/util/memory_output_stream.cpp @@ -46,12 +46,12 @@ MemoryOutputStream::MemoryOutputStream(const void* initial_data, size_t initial_ MemoryOutputStream::~MemoryOutputStream() {} -size_t MemoryOutputStream::Write(const void* data, size_t len) +bool MemoryOutputStream::Write(const void* data, size_t len) { const uint8_t* bytes = reinterpret_cast(data); buffer_.insert(buffer_.end(), bytes, bytes + len); - return len; + return true; } GFXRECON_END_NAMESPACE(util) diff --git a/framework/util/memory_output_stream.h b/framework/util/memory_output_stream.h index 0fd6b62345..ea28802b47 100644 --- a/framework/util/memory_output_stream.h +++ b/framework/util/memory_output_stream.h @@ -51,7 +51,7 @@ class MemoryOutputStream : public OutputStream virtual void Clear() { buffer_.clear(); }; - virtual size_t Write(const void* data, size_t len) override; + virtual bool Write(const void* data, size_t len) override; virtual const uint8_t* GetData() const { return buffer_.data(); } diff --git a/framework/util/output_stream.h b/framework/util/output_stream.h index d9026e6542..83526ffc11 100644 --- a/framework/util/output_stream.h +++ b/framework/util/output_stream.h @@ -39,7 +39,7 @@ class OutputStream virtual bool IsValid() { return false; } - virtual size_t Write(const void* data, size_t len) = 0; + virtual bool Write(const void* data, size_t len) = 0; virtual void Flush() {} }; diff --git a/framework/util/platform.h b/framework/util/platform.h index 598da69eb6..f82db57851 100644 --- a/framework/util/platform.h +++ b/framework/util/platform.h @@ -191,14 +191,14 @@ inline bool FileSeek(FILE* stream, int64_t offset, FileSeekOrigin origin) return (result == 0); } -inline size_t FileWriteNoLock(const void* buffer, size_t element_size, size_t element_count, FILE* stream) +inline bool FileWriteNoLock(const void* buffer, size_t bytes, FILE* stream) { - return _fwrite_nolock(buffer, element_size, element_count, stream); + return _fwrite_nolock(buffer, bytes, 1, stream) == 1; } -inline size_t FileReadNoLock(void* buffer, size_t element_size, size_t element_count, FILE* stream) +inline bool FileReadNoLock(void* buffer, size_t bytes, FILE* stream) { - return _fread_nolock(buffer, element_size, element_count, stream); + return _fread_nolock(buffer, bytes, 1, stream) == 1; } inline int32_t FileVprintf(FILE* stream, const char* format, va_list vlist) @@ -440,22 +440,36 @@ inline bool FileSeek(FILE* stream, int64_t offset, FileSeekOrigin origin) return (result == 0); } -inline size_t FileWriteNoLock(const void* buffer, size_t element_size, size_t element_count, FILE* stream) +inline bool FileWriteNoLock(const void* buffer, size_t bytes, FILE* stream) { + size_t write_count = 0; + int err = 0; + do + { #if defined(__APPLE__) || (defined(__ANDROID__) && (__ANDROID_API__ < 28)) - return fwrite(buffer, element_size, element_count, stream); + write_count = fwrite(buffer, bytes, 1, stream); #else - return fwrite_unlocked(buffer, element_size, element_count, stream); + write_count = fwrite_unlocked(buffer, bytes, 1, stream); #endif + err = ferror(stream); + } while (write_count < 1 && (err == EWOULDBLOCK || err == EINTR || err == EAGAIN)); + return (write_count == 1 || bytes == 0); } -inline size_t FileReadNoLock(void* buffer, size_t element_size, size_t element_count, FILE* stream) +inline bool FileReadNoLock(void* buffer, size_t bytes, FILE* stream) { + size_t read_count = 0; + int err = 0; + do + { #if defined(__APPLE__) || (defined(__ANDROID__) && (__ANDROID_API__ < 28)) - return fread(buffer, element_size, element_count, stream); + read_count = fread(buffer, bytes, 1, stream); #else - return fread_unlocked(buffer, element_size, element_count, stream); + read_count = fread_unlocked(buffer, bytes, 1, stream); #endif + err = ferror(stream); + } while (!feof(stream) && read_count < 1 && (err == EWOULDBLOCK || err == EINTR || err == EAGAIN)); + return (read_count == 1 || bytes == 0); } inline int32_t FileVprintf(FILE* stream, const char* format, va_list vlist) @@ -622,24 +636,38 @@ inline bool StringContains(const char* text, const char* substring) return strstr(text, substring) != nullptr; } -inline int32_t FilePuts(const char* char_string, FILE* stream) +inline int32_t FileFlush(FILE* stream) { - return fputs(char_string, stream); + return fflush(stream); } -inline int32_t FileFlush(FILE* stream) +inline bool FileWrite(const void* buffer, size_t bytes, FILE* stream) { - return fflush(stream); + size_t write_count = 0; + int err = 0; + do + { + write_count = fwrite(buffer, bytes, 1, stream); + err = ferror(stream); + } while (write_count < 1 && (err == EWOULDBLOCK || err == EINTR || err == EAGAIN)); + return (write_count == 1 || bytes == 0); } -inline size_t FileWrite(const void* buffer, size_t element_size, size_t element_count, FILE* stream) +inline bool FilePuts(const char* char_string, FILE* stream) { - return fwrite(buffer, element_size, element_count, stream); + return FileWrite(char_string, strlen(char_string), stream); } -inline size_t FileRead(void* buffer, size_t element_size, size_t element_count, FILE* stream) +inline bool FileRead(void* buffer, size_t bytes, FILE* stream) { - return fread(buffer, element_size, element_count, stream); + size_t read_count = 0; + int err = 0; + do + { + read_count = fread(buffer, bytes, 1, stream); + err = ferror(stream); + } while (!feof(stream) && read_count < 1 && (err == EWOULDBLOCK || err == EINTR || err == EAGAIN)); + return (read_count == 1 || bytes == 0); } inline int32_t SetFileBufferSize(FILE* stream, size_t buffer_size) diff --git a/tools/extract/main.cpp b/tools/extract/main.cpp index c1cdf1bb75..6f185b481e 100644 --- a/tools/extract/main.cpp +++ b/tools/extract/main.cpp @@ -133,8 +133,7 @@ class VulkanExtractConsumer : public gfxrecon::decode::VulkanConsumer int32_t result = gfxrecon::util::platform::FileOpen(&fp, file_path.c_str(), "wb"); if (result == 0) { - size_t written_size = gfxrecon::util::platform::FileWrite(orig_code, sizeof(char), orig_size, fp); - if (written_size != orig_size) + if (!gfxrecon::util::platform::FileWrite(orig_code, orig_size, fp)) { GFXRECON_WRITE_CONSOLE("Error while writing file %s: Could not complete", file_name.c_str()); }