Skip to content

Commit

Permalink
Do NOT call bitmap.close() in renderer worker (see description!)
Browse files Browse the repository at this point in the history
This fixes premature buffer destruction when using the COMBINATION of
foreign bitmap and zero-copy converter.
  • Loading branch information
mara004 committed May 13, 2023
1 parent e8ab921 commit 0cfe906
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 2 deletions.
1 change: 1 addition & 0 deletions docs/devel/changelog_staging.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,4 @@
<!-- List character: dash (-) -->

# Changelog for next release
- In `PdfDocument.render()`, fixed a bad `bitmap.close()` call that would lead to a downstream use after free when using the combination of foreign bitmap and no-copy conversion. Using foreign bitmaps was not the default and expressly not recommended.
9 changes: 8 additions & 1 deletion src/pypdfium2/_helpers/bitmap.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@ class PdfBitmap (AutoCloseable):
Note:
All attributes of :class:`.PdfBitmapInfo` are available in this class as well.
Warning:
``bitmap.close()``, which frees the buffer of foreign bitmaps, is not validated for safety.
A bitmap must not be closed when other objects still depend on its buffer!
Attributes:
raw (FPDF_BITMAP):
The underlying PDFium bitmap handle.
Expand Down Expand Up @@ -161,7 +165,10 @@ def fill_rect(self, left, top, width, height, color):
pdfium_c.FPDFBitmap_FillRect(self, left, top, width, height, c_color)


# Assumption: If the result is a view of the buffer, it holds a reference to said buffer (directly or indirectly), so that a possible finalizer is not called prematurely. This seems to hold true for numpy and PIL.
# Requirement: If the result is a view of the buffer (not a copy), it keeps the buffer's memory valid by retaining a reference to the buffer object.
# As of May 2023, this seems to hold true for numpy and PIL.
# TODO? Consider attaching a buffer keep-alive finalizer to the converted object


def to_numpy(self):
"""
Expand Down
5 changes: 4 additions & 1 deletion src/pypdfium2/_helpers/document.py
Original file line number Diff line number Diff line change
Expand Up @@ -550,7 +550,10 @@ def _process_page(cls, index, input_data, password, renderer, converter, pass_in
info = bitmap.get_info()
result = converter(bitmap)

for g in (bitmap, page, pdf):
# NOTE We MUST NOT call bitmap.close() before the converted object is serialized to the main process, otherwise we would free the buffer of a foreign bitmap prematurely if the converted object references the buffer rather than owning a copy. Confirmed by POC.
# This is not an issue when freeing the bitmap on garbage collection, provided the converted object keeps the buffer alive.

for g in (page, pdf):
g.close()

return (result, info) if pass_info else result
Expand Down

0 comments on commit 0cfe906

Please sign in to comment.