bblanchon / pdfium-binaries

📰 Binary distribution of PDFium
789 stars 166 forks source link

Consider disabling `use_allocator_shim` for apple builds #137

Closed HeavenVolkoff closed 6 months ago

HeavenVolkoff commented 6 months ago

Hello,

I have recently started using the binaries produced by this project, and I think I stumbled into an issue related to the use of the allocator_shim in the builds for macOS.

My use case is to generate PDF thumbnails in a rust application through this crate. However, I started receiving complaints, from apple users, about somewhat constant crashes when the app started processing multiple PDFs due to an internal assert failing in the allocator_shim code:

image

After some investigation (with help from @brxken128) I found the code triggering the assert:

https://chromium.googlesource.com/chromium/src/base/allocator/partition_allocator/+/refs/heads/main/src/partition_alloc/shim/allocator_shim.cc#129

Which is an apple specific code that tries to find the memory zones that handled a specific allocation and then free it.

However, this code can fail in a specific use case (which somewhat matches mine) due to an open issue: https://crbug.com/1271139:

image

and related hacky work-around:

https://chromium.googlesource.com/chromium/src/base/allocator/partition_allocator/+/refs/heads/main/src/partition_alloc/shim/allocator_shim_override_apple_default_zone.h#224

image

I think the best solution, while this issue is not solved upstream, is for the allocator_shim to be disabled for apple builds, like it is already disabled for the linux builds:

https://github.com/bblanchon/pdfium-binaries/blob/0d07b0d6c81e04e28d33c28604a405413e9a734f/steps/05-configure.sh#L28-L39

However, if this is not desired for some reason, I think adding a notice in the README for people that use this lib in apple systems with dlopen could be an alternative.

P.S.: Thank you for maintaining this project, it is great and really useful

bblanchon commented 6 months ago

Hi @HeavenVolkoff,

Thank you for reporting this issue; it sure sounds a like like #124. Please open a PR.

Also, consider creating a second PR to add a link to your project on the README.

Best Regards, Benoit

HeavenVolkoff commented 6 months ago

Hun, we are also using libheif in our project, just like #124. Although, I don't think this is the same problem as the assert we are triggering is in an Apple specific part of the allocator_shim code, but it is a funny coincidence. Anyway, @bblanchon I opened a PR for disabling the allocator_shim in macOS builds #138 and another to add Spacedrive to the related projects #139, thank you for the suggestion.