-
Notifications
You must be signed in to change notification settings - Fork 54
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Remove PdfPages::delete_page_at_index() in favour of PdfPage::delete(). #67
Comments
Added |
Added |
Actually, if |
Deprecated functions in |
As PDFium only provides a document-level page deleter, how did you implement this? |
Page indices of each |
I see. We can't do this in pypdfium2, as we also expose the raw pdfium API, so the caller might do operations that change page indices without helper classes knowing. |
I suspect it would be difficult to manage in a garbage-collected language because the point at which references are dropped is likely non-deterministic, making it difficult to know when to update any cache. Rust's memory model proves advantageous in this case. |
Sorry, I don't understand yet. What has this got to do with object references? |
When a caller calls However, like you say, you expose the raw Pdfium API, so the point is somewhat moot as I imagine you probably don't have a way of intercepting when callers are creating and releasing FPDF_* object references. |
Yes, the non-deterministic nature of garbage collection is a problem, see this section of pypdfium2's readme. However, I think we're kind of drifting away from the initial point. What I meant was just that a page object -> index mapping can be slightly dangerous if not implemented natively in PDFium. If there is any change to page order the mapping fails to track, then In pypdfium2 this is something we cannot do, as callers are encouraged to use helpers in conjunction with the raw API, and the raw API may potentially be used to do arbitrary modifications to page order. There's not much point in attempting to intercept raw API calls, because (a) we want to make the raw API available to the caller as-is (it would be confusing if calling a raw function would implicitly trigger something) (b) new APIs that change page order might be added to pdfium, so a page cache would imply the same risks, and (c) the purpose does not justify the complexity. |
If a reference to a
PdfPage
is retrieved usingPdfPages::get()
, then that page is deleted from the document usingPdfPages::delete_page_at_index()
, we still possess a (now invalid)PdfPage
reference. This is effectively a use-after-free violation. Avoid this by removing thePdfPages::delete_page_at_index()
function and instead use aPdfPage::delete()
function that consumesmut self
. This ensures the caller cannot hold an invalid reference to a page after it is deleted.The text was updated successfully, but these errors were encountered: