[billdenney]

Summary

This PR handles the leftovers from ADR-025 (PR 66, now merged). Base: main.

Two use-after-free families remained after ADR-025:

• Destroying PDFium with handles still open. cpp_destroy_library() called FPDF_DestroyLibrary() while documents and other handles were open. Closing, using or collecting any of them afterwards crashed R. That included doing so after the library was re-initialised by opening a document or by pdf_system_fonts_install_default(), and it included handles that were merely waiting for collection. .onUnload and two tests reach this path; no exported function does.
• Handles whose owner was deleted. A handle derived from a page-object passed validation while the thing it pinned (the page or the annotation) was still open, even after the object itself had been destroyed. Seven paths through exported functions read freed memory. One of them, a clip path after pdf_annot_delete(), segfaults without valgrind.

Two ADRs record the fixes:

• ADR-028: destroying the library closes every handle first. The ADR-025 registry gains the library as an owner. Documents, pdf_clip_path_new() clip paths, bitmaps and memory-document buffers are registered under it. cpp_destroy_library() releases them before FPDF_DestroyLibrary(), each document together with its own handles. make_handle() is now the only code that attaches a finalizer, and document_handle.h is folded into handle_registry.h.
• ADR-029: a handle is refused once any of its owners is closed. Each handle now pins its immediate owner: a clip path its page-object, a nested object its form. validate_handle() walks the whole prot chain, capped at 64 links; PDFium parses Form XObjects up to 40 levels deep, so the longest real chain is 44. is_open() follows the same chain in R. The closed-handle messages name the outermost owner that is closed.

Changes, by item

1. is_open() on an annotation's page-objects after pdf_annot_delete(). Confirmed at the R level only: the C++ layer already refused these handles, and valgrind showed 0 errors. Such objects now print [closed]. Page-object functions refuse them with "Parent annotation has been closed: it was deleted with pdf_annot_delete(). The object handle is no longer valid." The same applies to the children of forms among them. pdf_annot_update_object() and pdf_form_obj_remove_object() now check their page-object argument like every other page-object function.

2. pdf_obj_clip_path() on an annotation's object. Confirmed as a real use-after-free. An /AP /N stream with a clip before painting (q 10 10 50 50 re W n 1 0 0 rg 0 0 100 100 re f Q) gives the object a one-sub-path clip. After pdf_annot_delete(), pdf_clip_path_count() segfaulted. The fix pins the clip path to its page-object, and the chain walk refuses it. The six sibling paths (S1–S6 below) were swept here with the planner's approval.

3. cpp_destroy_library() with handles open. Fixed by releasing everything first (ADR-028), not by refusing. Refusing would depend on GC timing, and .onUnload cannot refuse anyway: runHook() turns the error into a warning. No exported function reaches cpp_destroy_library(). Opening a document, pdf_system_fonts_install_default(), and creating a clip path or bitmap now re-initialise the library.

4. pdf_form_fields() @return. It now says the function returns a pdfium_form_field_list of form-field handles. The tibble's columns are documented on as_tibble.pdfium_form_field_list(), which builds that tibble.

5. ADR-020 listed twice in dev/decisions/README.md. Not reproduced. Every commit that touched the index, every branch and every worktree lists ADR-020 exactly once, so the index is left alone. The duplicate most likely came from a transient merge state. Whoever resolves an ADR-index conflict should keep the 020 row only once. Its status cell differs across branches ("Accepted" vs "Accepted (§7 superseded by ADR-022)"), so a keep-both resolution duplicates it.

6. CLAUDE.md memory model. The section now covers:

   • which kinds are registered under a document and which under the library;
   • the single mint and release path;
   • that owners release first;
   • the weak entries;
   • the prot chain.

   It points at ADR-024, ADR-025, ADR-028 and ADR-029. No other section changed. dev/architecture.md and the architecture vignette are updated to match. The vignette also named a nonexistent library.unload() and claimed that quitting R runs .onUnload; both are corrected.

NEWS.md has a bullet under "Bug fixes" for each user-visible change.

Verification (picasso, Linux, R 4.6.1, valgrind 3.22, PDFium chromium/8066)

Each reproducer runs in its own process. "Before" is PR 66's branch at 2dfdb84; "after" is this branch. Numbers are valgrind invalid reads / writes / frees. "SIGSEGV" means the process died even under valgrind. The "Mismatched free()" reports inside libpdfium are the known noise and are not counted.

Scenario	Before	After
1 annotation objects after pdf_annot_delete()	0 / 0 / 0; is_open() TRUE, [open], internal C message	0 / 0 / 0; FALSE, [closed], "Parent annotation has been closed: …"
2 clip path of an annotation object after pdf_annot_delete()	2 / 0 / 0 (segfault without valgrind)	0 / 0 / 0, refused
S1 clip path after pdf_obj_delete(obj)	2 / 0 / 0	0 / 0 / 0
S2 form child after pdf_obj_delete(form)	5 / 0 / 0 (garbage bounds)	0 / 0 / 0
S3 clip path after pdf_form_obj_remove_object()	1 / 0 / 0	0 / 0 / 0
S4 clip path after pdf_annot_remove_object()	1 / 0 / 0	1 / 0 / 0, documented limitation (see below)
S5 child of a form moved into an annotation, after pdf_annot_delete()	1 / 0 / 0	0 / 0 / 0 (refused from the move on)
S6 clip path of an object moved into an annotation, after pdf_annot_delete()	1 / 0 / 0	0 / 0 / 0
3a pdf_doc_close() after destroy	1 / 0 / 0 + SIGSEGV (CPDF_FontGlobals::Clear)	0 / 0 / 0
3b re-init by opening a document, then close the old one	3 / 0 / 0 + SIGSEGV (FT_Done_Face)	0 / 0 / 0
3c re-init by pdf_system_fonts_install_default(), then gc()	3 / 0 / 0 + SIGSEGV	0 / 0 / 0
3d render after destroy	1 / 0 / 0 + SIGSEGV (CPDF_ColorSpace::GetStockCS)	0 / 0 / 0; "Page has been closed: its document was closed."
3e handles pending collection, destroy, re-init, gc()	3 / 0 / 0 + SIGSEGV	0 / 0 / 0
3f standalone clip path and bitmap across a destroy	0 / 0 / 0	0 / 0 / 0

Other checks on the final tree:

• Weekly CI valgrind job, exact invocation, serial, from tests/: TESTTHAT_PARALLEL=false R -d "valgrind --tool=memcheck --leak-check=full --num-callers=40 --error-exitcode=0 --log-file=<log>" --no-echo --vanilla -f testthat.R gives [ FAIL 0 | WARN 0 | SKIP 3 | PASS 3568 ] in 558 s. Definitely lost: 0 bytes; indirectly lost: 0 bytes; possibly lost: 0 bytes, so the CI gate passes. Invalid read / write / free: 0 / 0 / 0. There are 521 "Mismatched free()" contexts, all PDFium freeing its own allocations. One of them appears under cpp_destroy_library() only because FPDF_FreeDefaultSystemFontInfo() tail-calls operator delete on the block FPDF_GetDefaultSystemFontInfo() calloc'd.
• Full suite under MALLOC_CHECK_=3 with LD_PRELOAD=libc_malloc_debug.so.0: exit 0, [ FAIL 0 | WARN 0 | SKIP 3 | PASS 3568 ].
• Crash-isolated object sweep (65 functions × 8 appearance-stream and form targets × 2 teardown orders, MALLOC_CHECK_=3): 1040 of 1040 runs exit 0 and reach teardown. Every call has the same OK/ERR outcome and message as on the base branch, so no call is newly refused by these scenarios.
• Test suite: 3578 passed, 0 failed, 1 pre-existing skip.
• Coverage (covr, CI gate): R 100.00%. Every touched C++ file (handle_registry.cpp, init.cpp, clip.cpp, forms.cpp, api_completion.cpp, mutation.cpp, handle_validation.h) is also at 100%, counting its existing nocov exclusions.
• Lint and style: lintr reports 0 lints on the touched files. styler 1.10.3, the version the pre-commit hook pins, changes none of the lines this branch wrote, apart from eight lines in helper-inline-pdf.R; those use the same hanging indent as the existing builders in that file.
• R CMD check --as-cran: tests, examples, vignettes and "can be unloaded cleanly" are OK. The only findings come from the environment: 1 WARNING, because picasso lacks the checkbashisms script, and 1 NOTE, because Ubuntu's R compiles with -mno-omit-leaf-frame-pointer.
• Generated files: Rcpp::compileAttributes() and roxygen show no drift.

Known limitations, not fixed here

• Unloading the package with a handle still around crashes R later. .onUnload calls library.dynam.unload(). A handle that survives unloadNamespace() or detach(unload = TRUE) keeps a C finalizer pointing into the unmapped shared library. That holds even for a closed handle or one waiting for collection. R then crashes in R_RunWeakRefFinalizer() at the next gc() or at exit: exit 139 before and after this PR. pkgload::load_all() reloads hit it too. Per the user's decision the unload path is unchanged. A separate task (ADR-031) replaces the C finalizers with R-level finalizers that check the shared library is still loaded. ADR-028 keeps every finalizer in make_handle() so that change happens in one place.
• pdf_annot_remove_object() cannot reach handles made earlier (S4). It destroys an object by position, so those handles, and the clip paths and nested objects read through them, stay stale, as documented. The same holds for a second handle to one object, from a separate pdf_page_objects() or pdf_form_objects() call, after the object is deleted through the first. A possible fix, proposed and not implemented: register an annotation's page-object handles under the annotation in the ADR-025 registry, so pdf_annot_remove_object() can clear them.
• Conservative refusal after a move. After pdf_annot_append_object(annot, form), objects read through the form's (now closed) handle are refused, although they still exist inside the annotation. Read them again through pdf_annot_objects() and then pdf_form_objects().

🤖 Generated with Claude Code