search

ajrcarey / pdfium-render

A high-level idiomatic Rust wrapper around Pdfium, the C++ PDF library used by the Google Chromium project.
https://crates.io/crates/pdfium-render
Other
364 stars 59 forks source link

sync crate feature is unsafe to use with OnceCell #106

Closed bavardage closed 1 year ago

bavardage commented 1 year ago

demonstrated/described in https://github.com/ajrcarey/pdfium-render/pull/105

ajrcarey commented 1 year ago

Comments @bavardage submitted as part of PR #105, reproduced here verbatim:

I've been running into something similar in my usage of pdfium-render, and have attempted to reproduce via modifying the thread_safe example.

The readme alludes to it being ok, when using the sync feature, to initialize pdfium using something like a once_cell (I may have understood this part).

From reading the code though I don't see then how this would be protected - since the thread_safe feature seems to protect at bindings init time, but if you init once and then share around via sync feature there's nothing stopping concurrent access.

With this modified example I seem to reliably get segfaults when running this locally with cargo run --example thread_safe --features="thread_safe,sync"

In my actual usecase (where I'm using some other features of pdfium, but seem to reliably hit with text extraction) I see a mixture of problems:

text extraction is incorrect - missing characters/garbled output
format errors when loading valid documents
unknown pdfium errors (PdfiumLibraryInternalError)
../base/allocator/partition_allocator/partition_page.h(633) Check failed: entry != freelist_head
ajrcarey commented 1 year ago

Hi @bavardage , thank you for reporting the problem. If the documentation is misleading in any way, let's fix that; but other than making it abundantly clear that multi-threading with Pdfium is inherently unsafe and entirely at your own risk, I'm not sure what else I can do here.

I looked at your PR but it did not (as far I could see) provide any remedy (and I don't really think there is one for down-stream consumers of Pdfium), so I've closed it without action.

ajrcarey commented 1 year ago

Updated README to emphasise sync is not thread-safe. Will be included in release 0.8.9.