diff --git a/source/blender/blenlib/intern/BLI_mmap.cc b/source/blender/blenlib/intern/BLI_mmap.cc index e3c1ce9d43e..d69c4637468 100644 --- a/source/blender/blenlib/intern/BLI_mmap.cc +++ b/source/blender/blenlib/intern/BLI_mmap.cc @@ -7,20 +7,24 @@ */ #include "BLI_mmap.h" +#include "BLI_assert.h" #include "BLI_fileops.h" -#include "BLI_listbase.h" +#include "BLI_mutex.hh" +#include "BLI_string_utils.hh" +#include "BLI_vector.hh" #include "MEM_guardedalloc.h" +#include #include #ifndef WIN32 # include # include -# include /* For mmap. */ -# include /* For read close. */ +# include /* For `mmap`. */ +# include /* For `write`. */ #else # include "BLI_winstuff.h" -# include /* For open close read. */ +# include /* For `_get_osfhandle`. */ #endif struct BLI_mmap_file { @@ -31,72 +35,300 @@ struct BLI_mmap_file { size_t length; /* Platform-specific handle for the mapping. */ - void *handle; + void *volatile handle; /* Flag to indicate IO errors. Needs to be volatile since it's being set from * within the signal handler, which is not part of the normal execution flow. */ volatile bool io_error; + + /* Used to break out of infinite loops when an error keeps occurring. + * See the comments in #try_handle_error_for_address for details. */ + size_t id; }; -#ifndef WIN32 -/* When using memory-mapped files, any IO errors will result in a SIGBUS signal. - * Therefore, we need to catch that signal and stop reading the file in question. - * To do so, we keep a list of all current FileDatas that use memory-mapped files, - * and if a SIGBUS is caught, we check if the failed address is inside one of the - * mapped regions. - * If it is, we set a flag to indicate a failed read and remap the memory in - * question to a zero-backed region in order to avoid additional signals. - * The code that actually reads the memory area has to check whether the flag was - * set after it's done reading. - * If the error occurred outside of a memory-mapped region, we call the previous - * handler if one was configured and abort the process otherwise. - */ +/* General mutex used to protect access to the list of open mapped files, ensure the handler is + * initialized only once and to prevent multiple threads from trying to remap the same + * memory-mapped region in parallel. */ +static blender::Mutex mmap_mutex; -static struct error_handler_data { - ListBase open_mmaps; - char configured; - void (*next_handler)(int, siginfo_t *, void *); -} error_handler = {{nullptr}}; +/* When using memory-mapped files, any IO errors will result in an EXCEPTION_IN_PAGE_ERROR on + * Windows and a SIGBUS signal on other platforms. Therefore, we need to catch that signal and + * stop reading the file in question. To do so, we keep a list of all currently opened + * memory-mapped files, and if a error is caught, we check if the failed address is inside one of + * the mapped regions. If it is, we set a flag to indicate a failed read and remap the memory in + * question to a zero-backed region in order to avoid additional signals. The code that actually + * reads the memory area has to check whether the flag was set after it's done reading. If the + * error occurred outside of a memory-mapped region or the remapping failed, we call the previous + * handler if one was initialized and abort the process otherwise on Linux and on Windows let the + * exception crash the program. */ +static blender::Vector &open_mmaps_vector() +{ + static blender::Vector open_mmaps; + return open_mmaps; +} -static void sigbus_handler(int sig, siginfo_t *siginfo, void *ptr) +/* Print a message to the STDERR without using the standard library routines. + * If a MMAP error occurs while reading a pointer inside one of the standard library's IO routines, + * any global locks it was holding won't be unlocked when entering the handler. + * Using the normal printing routines could then cause a deadlock. */ +static void print_error(const char *message); + +/* Tries to replace the mapping with zeroes. + * Returns true on success. */ +static bool try_map_zeros(BLI_mmap_file *file); + +/* Find the file mapping containing the address and call #try_map_zeroes for it. + * Returns true when execution can continue. */ +static bool try_handle_error_for_address(const void *address) +{ + static thread_local size_t last_handled_file_id = -1; + + std::unique_lock lock(mmap_mutex); + + BLI_mmap_file *file = nullptr; + for (BLI_mmap_file *link_file : open_mmaps_vector()) { + /* Is the address where the error occurred in this file's mapped range? */ + if (address >= link_file->memory && address < link_file->memory + link_file->length) { + file = link_file; + break; + } + } + + if (file == nullptr) { + /* Not our error. */ + return false; + } + + /* Check if we already handled this error. */ + if (file->io_error) { + /* If `file->io_error` is true, either a different thread has + * already replaced the mapping after this thread raised the + * exception, but before we got the lock, and execution can + * continue, or replacing the mapping did not avoid the current + * exception. We need to check if continuing execution fails to + * avoid an infinite loop in the second case. To detect such a + * situation, the last handled mapping's ID is stored per thread and + * compared against it to see if continuing execution was already + * tried for this mapping in this thread. If that is the case, + * forward the exception instead of continuing execution again. As + * multiple threads could encounter an exception for the same + * mapping at the same time, a boolean stored in `BLI_mmap_file` + * would not work for this detection, as the condition we need to + * detect is thread dependent. */ + if (file->id == last_handled_file_id) { + /* Some possible causes of the error below are: + * - Thread safety issues in the error handling code. + * - Faulty remapping without having signaled an error in `try_map_zeros`. + * - Invalid usage of an address in the mapped range, such as + * unaligned access on some platforms. + */ + print_error( + "Error: Unexpected exception in mapped file which was already remapped with zeros."); + return false; + } + /* Another thread has already remapped the range, we can continue execution. */ + last_handled_file_id = file->id; + return true; + } + + last_handled_file_id = file->id; + file->io_error = true; + + if (!try_map_zeros(file)) { + print_error("Error: Could not replace mapped file with zeros."); + return false; + } + + return true; +} + +#ifdef WIN32 +using MapViewOfFile3Fn = PVOID(WINAPI *)(HANDLE FileMapping, + HANDLE Process, + PVOID BaseAddress, + ULONG64 Offset, + SIZE_T ViewSize, + ULONG AllocationType, + ULONG PageProtection, + MEM_EXTENDED_PARAMETER *ExtendedParameters, + ULONG ParameterCount); + +using VirtualAlloc2Fn = PVOID(WINAPI *)(HANDLE Process, + PVOID BaseAddress, + SIZE_T Size, + ULONG AllocationType, + ULONG PageProtection, + MEM_EXTENDED_PARAMETER *ExtendedParameters, + ULONG ParameterCount); + +/* Pointers to `MapViewOfFile3` and `VirtualAlloc2`, as they need to be dynamically linked + * at run-time because they are only available on Windows 10 (1803) or newer. + * If they are not available, error handling is not used. */ +static MapViewOfFile3Fn mmap_MapViewOfFile3 = nullptr; + +static VirtualAlloc2Fn mmap_VirtualAlloc2 = nullptr; + +static void print_error(const char *message) +{ + char buffer[256]; + size_t length = BLI_string_join(buffer, sizeof(buffer), "BLI_mmap: ", message, "\r\n"); + HANDLE stderr_handle = GetStdHandle(STD_ERROR_HANDLE); + WriteFile(stderr_handle, buffer, length, nullptr, nullptr); +} + +static bool try_map_zeros(BLI_mmap_file *file) +{ + if (!UnmapViewOfFileEx(file->memory, MEM_PRESERVE_PLACEHOLDER)) { + return false; + } + + if (!CloseHandle(file->handle)) { + return false; + } + + ULARGE_INTEGER length_ularge_int; + length_ularge_int.QuadPart = file->length; + file->handle = CreateFileMapping(INVALID_HANDLE_VALUE, + nullptr, + PAGE_READONLY, + length_ularge_int.HighPart, + length_ularge_int.LowPart, + nullptr); + if (file->handle == nullptr) { + return false; + } + + void *memory = mmap_MapViewOfFile3(file->handle, + nullptr, + file->memory, + 0, + file->length, + MEM_REPLACE_PLACEHOLDER, + PAGE_READONLY, + nullptr, + 0); + if (memory == nullptr) { + return false; + } + + BLI_assert(memory == file->memory); + + return true; +} + +static LONG page_exception_handler(EXCEPTION_POINTERS *ExceptionInfo) noexcept +{ + /* On Windows, if an IO error occurs trying to read from a mapped file, an + * EXCEPTION_IN_PAGE_ERROR error will be raised. Also check for + * EXCEPTION_ACCESS_VIOLATION, which can be raised when a thread tries to read from the mapping + * while it is being replaced by another. */ + if (ExceptionInfo->ExceptionRecord->ExceptionCode == EXCEPTION_IN_PAGE_ERROR || + ExceptionInfo->ExceptionRecord->ExceptionCode == EXCEPTION_ACCESS_VIOLATION) + { + if (ExceptionInfo->ExceptionRecord->NumberParameters >= 2) { + /* Currently, MMAP'd files are read only, so don't replace the mapping when a write is + * attempted. */ + if (ExceptionInfo->ExceptionRecord->ExceptionInformation[0] == 1) { + return EXCEPTION_CONTINUE_SEARCH; + } + const void *address = reinterpret_cast( + ExceptionInfo->ExceptionRecord->ExceptionInformation[1]); + if (try_handle_error_for_address(address)) { + return EXCEPTION_CONTINUE_EXECUTION; + } + } + } + return EXCEPTION_CONTINUE_SEARCH; +} + +/* Ensures that the error handler is set up and ready. */ +static bool ensure_mmap_initialized() +{ + static std::atomic_bool initialized = false; + if (initialized) { + return true; + } + + std::unique_lock lock(mmap_mutex); + + if (!initialized) { + HMODULE kernelbase = ::LoadLibraryA("kernelbase.dll"); + if (kernelbase) { + mmap_MapViewOfFile3 = reinterpret_cast( + ::GetProcAddress(kernelbase, "MapViewOfFile3")); + mmap_VirtualAlloc2 = reinterpret_cast( + ::GetProcAddress(kernelbase, "VirtualAlloc2")); + } + if (mmap_MapViewOfFile3 && mmap_VirtualAlloc2) { + /* First has to be FALSE to avoid our handler being called before ASAN's handler. */ + AddVectoredExceptionHandler(FALSE, page_exception_handler); + } + else { + print_error("Could not load necessary functions for MMAP error handling."); + } + initialized = true; + } + return true; +} +#else /* !WIN32 */ +static void print_error(const char *message) +{ + char buffer[256]; + size_t length = BLI_string_join(buffer, sizeof(buffer), "BLI_mmap: ", message, "\n"); + write(STDERR_FILENO, buffer, length); +} + +static bool try_map_zeros(BLI_mmap_file *file) +{ + /* Replace the mapped memory with zeroes. */ + const void *mapped_memory = mmap( + file->memory, file->length, PROT_READ, MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0); + if (mapped_memory == MAP_FAILED) { + return false; + } + + return true; +} + +static struct sigaction next_handler = {}; + +static void sigbus_handler(int sig, siginfo_t *siginfo, void *ptr) noexcept { /* We only handle SIGBUS here for now. */ BLI_assert(sig == SIGBUS); - const char *error_addr = (const char *)siginfo->si_addr; - /* Find the file that this error belongs to. */ - LISTBASE_FOREACH (LinkData *, link, &error_handler.open_mmaps) { - BLI_mmap_file *file = static_cast(link->data); - - /* Is the address where the error occurred in this file's mapped range? */ - if (error_addr >= file->memory && error_addr < file->memory + file->length) { - file->io_error = true; - - /* Replace the mapped memory with zeroes. */ - const void *mapped_memory = mmap( - file->memory, file->length, PROT_READ, MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0); - if (mapped_memory == MAP_FAILED) { - fprintf(stderr, "SIGBUS handler: Error replacing mapped file with zeros\n"); - } - - return; - } + if (try_handle_error_for_address(siginfo->si_addr)) { + return; } - /* Fall back to other handler if there was one. */ - if (error_handler.next_handler) { - error_handler.next_handler(sig, siginfo, ptr); + /* Fall back to the other handler if there was one. + * + * No lock is needed here, as #try_handle_error_for_address + * unconditionally locks `mmap_mutex`, and as such + * #ensure_mmap_initialized must have finished and #next_handler + * will be set up. */ + if (next_handler.sa_sigaction && (next_handler.sa_flags & SA_SIGINFO)) { + next_handler.sa_sigaction(sig, siginfo, ptr); + } + else if (!ELEM(next_handler.sa_handler, nullptr, SIG_DFL, SIG_IGN)) { + next_handler.sa_handler(sig); } else { - fprintf(stderr, "Unhandled SIGBUS caught\n"); + print_error("Unhandled SIGBUS caught"); abort(); } } /* Ensures that the error handler is set up and ready. */ -static bool sigbus_handler_setup() +static bool ensure_mmap_initialized() { - if (!error_handler.configured) { + static std::atomic_bool initialized = false; + if (initialized) { + return true; + } + + std::unique_lock lock(mmap_mutex); + if (!initialized) { struct sigaction newact = {{nullptr}}, oldact = {{nullptr}}; newact.sa_sigaction = sigbus_handler; @@ -106,76 +338,118 @@ static bool sigbus_handler_setup() return false; } - /* Remember the previously configured handler to fall back to it if the error + /* Remember the previous handler to fall back to it if the error * does not belong to any of the mapped files. */ - error_handler.next_handler = oldact.sa_sigaction; - error_handler.configured = 1; + next_handler = oldact; + initialized = true; } return true; } +#endif /* !WIN32 */ /* Adds a file to the list that the error handler checks. */ -static void sigbus_handler_add(BLI_mmap_file *file) +static void error_handler_add(BLI_mmap_file *file) { - BLI_addtail(&error_handler.open_mmaps, BLI_genericNodeN(file)); + std::unique_lock lock(mmap_mutex); + open_mmaps_vector().append(file); } /* Removes a file from the list that the error handler checks. */ -static void sigbus_handler_remove(BLI_mmap_file *file) +static void error_handler_remove(BLI_mmap_file *file) { - LinkData *link = static_cast( - BLI_findptr(&error_handler.open_mmaps, file, offsetof(LinkData, data))); - BLI_freelinkN(&error_handler.open_mmaps, link); + std::unique_lock lock(mmap_mutex); + open_mmaps_vector().remove_first_occurrence_and_reorder(file); } -#endif BLI_mmap_file *BLI_mmap_open(int fd) { + static std::atomic_size_t id_counter = 0; + void *memory, *handle = nullptr; const size_t length = BLI_lseek(fd, 0, SEEK_END); if (UNLIKELY(length == size_t(-1))) { return nullptr; } -#ifndef WIN32 - /* Ensure that the SIGBUS handler is configured. */ - if (!sigbus_handler_setup()) { + /* Ensures that the error handler is set up and ready. */ + if (!ensure_mmap_initialized()) { return nullptr; } +#ifndef WIN32 /* Map the given file to memory. */ memory = mmap(nullptr, length, PROT_READ, MAP_PRIVATE, fd, 0); if (memory == MAP_FAILED) { return nullptr; } -#else +#else /* WIN32 */ /* Convert the POSIX-style file descriptor to a Windows handle. */ void *file_handle = (void *)_get_osfhandle(fd); - /* Memory mapping on Windows is a two-step process - first we create a mapping, - * then we create a view into that mapping. - * In our case, one view that spans the entire file is enough. */ - handle = CreateFileMapping(file_handle, nullptr, PAGE_READONLY, 0, 0, nullptr); - if (handle == nullptr) { - return nullptr; - } - memory = MapViewOfFile(handle, FILE_MAP_READ, 0, 0, 0); - if (memory == nullptr) { - CloseHandle(handle); - return nullptr; - } -#endif - /* Now that the mapping was successful, allocate memory and set up the BLI_mmap_file. */ + /* Memory mapping on Windows is a multi-step process - first we create a placeholder + * allocation. Then we create a mapping, and after that we create a view into that mapping + * on top of the placeholder. In our case, one view that spans the entire file is enough. + * NOTE: Changes to protection flags should also be reflected in #try_map_zeros. If write + * support is added, the write check in #page_exception_handler should be updated. */ + if (mmap_MapViewOfFile3 && mmap_VirtualAlloc2) { + memory = mmap_VirtualAlloc2(nullptr, + nullptr, + length, + MEM_RESERVE | MEM_RESERVE_PLACEHOLDER, + PAGE_NOACCESS, + nullptr, + 0); + if (memory == nullptr) { + return nullptr; + } + + handle = CreateFileMapping(file_handle, nullptr, PAGE_READONLY, 0, 0, nullptr); + if (handle == nullptr) { + VirtualFree(memory, 0, MEM_RELEASE); + return nullptr; + } + + if (mmap_MapViewOfFile3(handle, + nullptr, + memory, + 0, + length, + MEM_REPLACE_PLACEHOLDER, + PAGE_READONLY, + nullptr, + 0) == nullptr) + { + VirtualFree(memory, 0, MEM_RELEASE); + CloseHandle(handle); + return nullptr; + } + } + else { + /* Fallback without error handling in case `MapViewOfFile3` or `VirtualAlloc2` is not + * available. */ + handle = CreateFileMapping(file_handle, nullptr, PAGE_READONLY, 0, 0, nullptr); + if (handle == nullptr) { + return nullptr; + } + + memory = MapViewOfFile(handle, FILE_MAP_READ, 0, 0, 0); + if (memory == nullptr) { + CloseHandle(handle); + return nullptr; + } + } +#endif /* WIN32 */ + + /* Now that the mapping was successful, allocate memory and set up the #BLI_mmap_file. */ BLI_mmap_file *file = MEM_callocN(__func__); file->memory = static_cast(memory); file->handle = handle; file->length = length; + file->id = id_counter++; -#ifndef WIN32 /* Register the file with the error handler. */ - sigbus_handler_add(file); -#endif + error_handler_add(file); return file; } @@ -188,23 +462,7 @@ bool BLI_mmap_read(BLI_mmap_file *file, void *dest, size_t offset, size_t length return false; } -#ifndef WIN32 - /* If an error occurs in this call, sigbus_handler will be called and will set - * file->io_error to true. */ memcpy(dest, file->memory + offset, length); -#else - /* On Windows, we use exception handling to be notified of errors. */ - __try - { - memcpy(dest, file->memory + offset, length); - } - __except (GetExceptionCode() == EXCEPTION_IN_PAGE_ERROR ? EXCEPTION_EXECUTE_HANDLER : - EXCEPTION_CONTINUE_SEARCH) - { - file->io_error = true; - return false; - } -#endif return !file->io_error; } @@ -226,9 +484,9 @@ bool BLI_mmap_any_io_error(const BLI_mmap_file *file) void BLI_mmap_free(BLI_mmap_file *file) { + error_handler_remove(file); #ifndef WIN32 munmap((void *)file->memory, file->length); - sigbus_handler_remove(file); #else UnmapViewOfFile(file->memory); CloseHandle(file->handle); diff --git a/source/blender/imbuf/intern/IMB_allocimbuf.hh b/source/blender/imbuf/intern/IMB_allocimbuf.hh index af469d858b8..c97d6fc94d7 100644 --- a/source/blender/imbuf/intern/IMB_allocimbuf.hh +++ b/source/blender/imbuf/intern/IMB_allocimbuf.hh @@ -9,17 +9,5 @@ struct ImBuf; -#ifndef WIN32 -void imb_mmap_lock_init(); -void imb_mmap_lock_exit(); -void imb_mmap_lock(); -void imb_mmap_unlock(); -#else -# define imb_mmap_lock_init() -# define imb_mmap_lock_exit() -# define imb_mmap_lock() -# define imb_mmap_unlock() -#endif - bool imb_addencodedbufferImBuf(ImBuf *ibuf); bool imb_enlargeencodedbufferImBuf(ImBuf *ibuf); diff --git a/source/blender/imbuf/intern/allocimbuf.cc b/source/blender/imbuf/intern/allocimbuf.cc index 188030298b9..c844e33df6a 100644 --- a/source/blender/imbuf/intern/allocimbuf.cc +++ b/source/blender/imbuf/intern/allocimbuf.cc @@ -34,30 +34,6 @@ static CLG_LogRef LOG = {"image.buffer"}; -#ifndef WIN32 -static SpinLock mmap_spin; - -void imb_mmap_lock_init() -{ - BLI_spin_init(&mmap_spin); -} - -void imb_mmap_lock_exit() -{ - BLI_spin_end(&mmap_spin); -} - -void imb_mmap_lock() -{ - BLI_spin_lock(&mmap_spin); -} - -void imb_mmap_unlock() -{ - BLI_spin_unlock(&mmap_spin); -} -#endif - /* Free the specified buffer storage, freeing memory when needed and restoring the state of the * buffer to its defaults. */ template static void imb_free_buffer(BufferType &buffer) diff --git a/source/blender/imbuf/intern/format_webp.cc b/source/blender/imbuf/intern/format_webp.cc index 880fb4f105b..d2dfb914594 100644 --- a/source/blender/imbuf/intern/format_webp.cc +++ b/source/blender/imbuf/intern/format_webp.cc @@ -89,9 +89,7 @@ ImBuf *imb_load_filepath_thumbnail_webp(const char *filepath, return nullptr; } - imb_mmap_lock(); BLI_mmap_file *mmap_file = BLI_mmap_open(file); - imb_mmap_unlock(); close(file); if (mmap_file == nullptr) { return nullptr; @@ -102,12 +100,11 @@ ImBuf *imb_load_filepath_thumbnail_webp(const char *filepath, WebPDecoderConfig config; if (!data || !WebPInitDecoderConfig(&config) || - WebPGetFeatures(data, data_size, &config.input) != VP8_STATUS_OK) + WebPGetFeatures(data, data_size, &config.input) != VP8_STATUS_OK || + BLI_mmap_any_io_error(mmap_file)) { CLOG_ERROR(&LOG, "Invalid file"); - imb_mmap_lock(); BLI_mmap_free(mmap_file); - imb_mmap_unlock(); return nullptr; } @@ -122,9 +119,7 @@ ImBuf *imb_load_filepath_thumbnail_webp(const char *filepath, ImBuf *ibuf = IMB_allocImBuf(dest_w, dest_h, 32, IB_byte_data); if (ibuf == nullptr) { CLOG_ERROR(&LOG, "Failed to allocate image memory"); - imb_mmap_lock(); BLI_mmap_free(mmap_file); - imb_mmap_unlock(); return nullptr; } @@ -141,22 +136,17 @@ ImBuf *imb_load_filepath_thumbnail_webp(const char *filepath, config.output.u.RGBA.stride = 4 * ibuf->x; config.output.u.RGBA.size = size_t(config.output.u.RGBA.stride) * size_t(ibuf->y); - if (WebPDecode(data, data_size, &config) != VP8_STATUS_OK) { + if (WebPDecode(data, data_size, &config) != VP8_STATUS_OK || BLI_mmap_any_io_error(mmap_file)) { CLOG_ERROR(&LOG, "Failed to decode image"); IMB_freeImBuf(ibuf); - - imb_mmap_lock(); BLI_mmap_free(mmap_file); - imb_mmap_unlock(); return nullptr; } /* Free the output buffer. */ WebPFreeDecBuffer(&config.output); - imb_mmap_lock(); BLI_mmap_free(mmap_file); - imb_mmap_unlock(); return ibuf; } diff --git a/source/blender/imbuf/intern/module.cc b/source/blender/imbuf/intern/module.cc index e47a5562c30..541143be99a 100644 --- a/source/blender/imbuf/intern/module.cc +++ b/source/blender/imbuf/intern/module.cc @@ -8,14 +8,12 @@ #include -#include "IMB_allocimbuf.hh" #include "IMB_colormanagement_intern.hh" #include "IMB_filetype.hh" #include "IMB_imbuf.hh" void IMB_init() { - imb_mmap_lock_init(); imb_filetypes_init(); colormanagement_init(); } @@ -24,5 +22,4 @@ void IMB_exit() { imb_filetypes_exit(); colormanagement_exit(); - imb_mmap_lock_exit(); } diff --git a/source/blender/imbuf/intern/openexr/openexr_api.cpp b/source/blender/imbuf/intern/openexr/openexr_api.cpp index 98f46a578f6..3cc3a009719 100644 --- a/source/blender/imbuf/intern/openexr/openexr_api.cpp +++ b/source/blender/imbuf/intern/openexr/openexr_api.cpp @@ -182,22 +182,17 @@ class IMMapStream : public Imf::IStream { throw IEX_NAMESPACE::InputExc("file not found"); } _exrpos = 0; - imb_mmap_lock(); _mmap_file = BLI_mmap_open(file); - imb_mmap_unlock(); close(file); if (_mmap_file == nullptr) { throw IEX_NAMESPACE::InputExc("BLI_mmap_open failed"); } - _exrbuf = (uchar *)BLI_mmap_get_pointer(_mmap_file); _exrsize = BLI_mmap_get_length(_mmap_file); } ~IMMapStream() override { - imb_mmap_lock(); BLI_mmap_free(_mmap_file); - imb_mmap_unlock(); } /* This is implementing regular `read`, not `readMemoryMapped`, because DWAA and DWAB @@ -208,7 +203,11 @@ class IMMapStream : public Imf::IStream { if (_exrpos + n > _exrsize) { throw Iex::InputExc("Unexpected end of file."); } - memcpy(c, _exrbuf + _exrpos, n); + + if (!BLI_mmap_read(_mmap_file, c, _exrpos, n)) { + throw Iex::InputExc("Error reading file."); + } + _exrpos += n; return _exrpos < _exrsize; @@ -228,7 +227,6 @@ class IMMapStream : public Imf::IStream { BLI_mmap_file *_mmap_file; exr_file_offset_t _exrpos; exr_file_offset_t _exrsize; - uchar *_exrbuf; }; /* File Input Stream */ diff --git a/source/blender/imbuf/intern/readimage.cc b/source/blender/imbuf/intern/readimage.cc index bf93dc6d5be..94e92e50d5d 100644 --- a/source/blender/imbuf/intern/readimage.cc +++ b/source/blender/imbuf/intern/readimage.cc @@ -7,8 +7,6 @@ */ #ifdef _WIN32 -# include "BLI_winstuff.h" -# include # include # include # include @@ -159,15 +157,13 @@ ImBuf *IMB_load_image_from_file_descriptor(const int file, const char *filepath, char r_colorspace[IM_MAX_SPACE]) { - ImBuf *ibuf; + ImBuf *ibuf = nullptr; if (file == -1) { return nullptr; } - imb_mmap_lock(); BLI_mmap_file *mmap_file = BLI_mmap_open(file); - imb_mmap_unlock(); if (mmap_file == nullptr) { CLOG_ERROR(&LOG, "%s: couldn't get mapping for \"%s\"", __func__, filepath); return nullptr; @@ -176,34 +172,16 @@ ImBuf *IMB_load_image_from_file_descriptor(const int file, const uchar *mem = static_cast(BLI_mmap_get_pointer(mmap_file)); const size_t size = BLI_mmap_get_length(mmap_file); - /* There could be broken mmap due to network drives and other issues, handles exception the - * same way as in #BLI_mmap_read. Note that if the mmap becomes invalid mid-way through reading, - * external calls in #IMB_load_image_from_memory could leave unfreed memory, but this is the - * limitation of current exception handling method. Ref #139472. */ -#ifdef WIN32 - __try - { -#endif + ibuf = IMB_load_image_from_memory(mem, size, flags, filepath, filepath, r_colorspace); - ibuf = IMB_load_image_from_memory(mem, size, flags, filepath, filepath, r_colorspace); - -#ifdef WIN32 - } - __except (GetExceptionCode() == EXCEPTION_IN_PAGE_ERROR ? EXCEPTION_EXECUTE_HANDLER : - EXCEPTION_CONTINUE_SEARCH) - { + /* If we got an image but mmap encountered an error, + * free the image and return nullptr as it could be corrupted. */ + if (ibuf != nullptr && BLI_mmap_any_io_error(mmap_file)) { + IMB_freeImBuf(ibuf); ibuf = nullptr; } -#else - /* For unix, if mmap encounters an exception, BLI_mmap_file::io_error would be set. */ - if (BLI_mmap_any_io_error(mmap_file)) { - ibuf = nullptr; - } -#endif - imb_mmap_lock(); BLI_mmap_free(mmap_file); - imb_mmap_unlock(); return ibuf; }