From f87f6b65fa92aec2354be1bed3d52f9efe9f0d47 Mon Sep 17 00:00:00 2001 From: Bob Singor Date: Tue, 2 Jun 2026 23:24:34 +0300 Subject: [PATCH 1/7] Add optional thread-local globals and thread-soak test Introduce optional per-thread PDFium globals to support a thread-confined runtime for server worker pools. Adds core/fxcrt/epdf_tls.h (EPDF_TLS) and converts numerous process-global singletons to be thread-local when embedpdf_thread_local_globals is enabled. Exposes EPDF_InitThread/EPDF_ShutdownThread as lifecycle entry points, makes localtime usage thread-safe, and adjusts RNG/errno/timer/render/color/font/module globals accordingly. Adds a thread-soak test harness (testing/tools:epdf_thread_soak), build/test scripts (scripts/embedpdf-runtime/*), GN flag (embedpdf_thread_local_globals) and a GitHub Actions workflow (pdfium-tsan-thread-soak.yml) to run the TSAN-backed soak. Default behavior remains unchanged (flag off) so builds are byte-for-byte compatible unless explicitly enabled. --- .github/workflows/pdfium-tsan-thread-soak.yml | 79 ++++++++ BUILD.gn | 11 ++ core/fpdfapi/edit/cpdf_creator.cpp | 11 +- core/fpdfapi/font/cpdf_fontglobals.cpp | 5 +- core/fpdfapi/page/cpdf_colorspace.cpp | 5 +- .../fpdfapi/page/cpdf_streamcontentparser.cpp | 5 +- core/fpdfapi/render/cpdf_renderstatus.cpp | 5 +- core/fxcodec/icc/icc_transform.cpp | 16 ++ core/fxcrt/cfx_timer.cpp | 7 +- core/fxcrt/epdf_tls.h | 30 +++ core/fxcrt/fx_extension.cpp | 16 +- core/fxcrt/fx_random.cpp | 7 +- core/fxcrt/fx_system.cpp | 5 +- core/fxge/cfx_gemodule.cpp | 6 +- fpdfsdk/BUILD.gn | 1 + fpdfsdk/cpdfsdk_helpers.cpp | 12 ++ fpdfsdk/epdf_threading.cpp | 30 +++ fpdfsdk/fpdf_view.cpp | 5 +- fpdfsdk/fpdf_view_c_api_test.c | 2 + pdfium.gni | 8 + public/fpdf_ext.h | 5 +- public/fpdfview.h | 18 ++ scripts/embedpdf-runtime/build-target.sh | 12 ++ scripts/embedpdf-runtime/test-target.sh | 6 + .../embedpdf-runtime/thread-soak-target.sh | 101 ++++++++++ testing/tools/BUILD.gn | 14 ++ testing/tools/epdf_thread_soak.cpp | 187 ++++++++++++++++++ 27 files changed, 594 insertions(+), 15 deletions(-) create mode 100644 .github/workflows/pdfium-tsan-thread-soak.yml create mode 100644 core/fxcrt/epdf_tls.h create mode 100644 fpdfsdk/epdf_threading.cpp create mode 100755 scripts/embedpdf-runtime/thread-soak-target.sh create mode 100644 testing/tools/epdf_thread_soak.cpp diff --git a/.github/workflows/pdfium-tsan-thread-soak.yml b/.github/workflows/pdfium-tsan-thread-soak.yml new file mode 100644 index 0000000000..e4347fc457 --- /dev/null +++ b/.github/workflows/pdfium-tsan-thread-soak.yml @@ -0,0 +1,79 @@ +name: PDFium TSAN Thread Soak + +on: + workflow_dispatch: + inputs: + ref: + description: Git ref to test + required: false + default: embedpdf/main + pdf_path: + description: PDF fixture to render during the soak + required: false + default: testing/resources/hello_world.pdf + threads: + description: Number of worker threads + required: false + default: '2' + iterations: + description: Iterations per worker thread + required: false + default: '2' + +permissions: + contents: read + +jobs: + tsan-thread-soak: + name: Linux x64 TSAN thread soak + runs-on: ubuntu-24.04 + timeout-minutes: 90 + env: + PDF_RUNTIME_SYNC: auto + PDF_RUNTIME_TARGET_OS_LIST: linux + EMBEDPDF_TLS_GLOBALS: true + EMBEDPDF_TSAN: 1 + PDF_PATH: ${{ inputs.pdf_path }} + SOAK_THREADS: ${{ inputs.threads }} + SOAK_ITERATIONS: ${{ inputs.iterations }} + SOAK_OUT: out/embedpdf-runtime-thread-soak-linux-x64 + + steps: + - uses: actions/checkout@v6 + with: + ref: ${{ inputs.ref || github.ref_name }} + + - name: Install depot_tools + shell: bash + run: | + git clone https://chromium.googlesource.com/chromium/tools/depot_tools.git "$RUNNER_TEMP/depot_tools" + echo "$RUNNER_TEMP/depot_tools" >> "$GITHUB_PATH" + + - name: Install Linux base deps + shell: bash + run: | + sudo apt-get update + sudo apt-get install -y --no-install-recommends cmake clang lld curl g++ ninja-build pkg-config tar + + - name: Run TSAN thread soak + shell: bash + run: | + set -euo pipefail + mkdir -p "$SOAK_OUT" + scripts/embedpdf-runtime/thread-soak-target.sh \ + linux-x64 \ + "$PDF_PATH" \ + -- \ + --threads="$SOAK_THREADS" \ + --iterations="$SOAK_ITERATIONS" \ + 2>&1 | tee "$SOAK_OUT/thread-soak.log" + + - name: Upload TSAN logs + if: always() + uses: actions/upload-artifact@v6 + with: + name: pdfium-tsan-thread-soak-logs + path: | + out/embedpdf-runtime-thread-soak-linux-x64/thread-soak.log + out/embedpdf-runtime-thread-soak-linux-x64/args.gn + if-no-files-found: ignore diff --git a/BUILD.gn b/BUILD.gn index fbdb13a4e3..3ba3799196 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -51,6 +51,13 @@ config("pdfium_common_config") { defines += [ "PDF_USE_PARTITION_ALLOC" ] } + # EmbedPDF: thread-confined runtime. Translates the embedpdf_thread_local_globals + # GN arg into the EPDF_THREAD_LOCAL_GLOBALS define consumed by EPDF_TLS + # (core/fxcrt/epdf_tls.h). Default off; see pdfium.gni. + if (embedpdf_thread_local_globals) { + defines += [ "EPDF_THREAD_LOCAL_GLOBALS" ] + } + if (is_win) { # Assume UTF-8 by default to avoid code page dependencies. cflags += [ "/utf-8" ] @@ -191,6 +198,7 @@ source_set("pdfium_public_headers_impl") { sources = [ "public/cpp/fpdf_deleters.h", "public/cpp/fpdf_scopers.h", + "public/epdf_redact.h", "public/fpdf_annot.h", "public/fpdf_attachment.h", "public/fpdf_catalog.h", @@ -431,6 +439,9 @@ group("pdfium_all") { ":pdfium_unittests", "testing:pdfium_test", "testing/fuzzers", + "testing/tools:epdf_layer_memory_benchmark", + "testing/tools:epdf_layer_replay_soak", + "testing/tools:epdf_thread_soak", ] if (pdf_is_standalone) { diff --git a/core/fpdfapi/edit/cpdf_creator.cpp b/core/fpdfapi/edit/cpdf_creator.cpp index 2dee5f5aa8..8346b2a278 100644 --- a/core/fpdfapi/edit/cpdf_creator.cpp +++ b/core/fpdfapi/edit/cpdf_creator.cpp @@ -545,6 +545,11 @@ CPDF_Creator::Stage CPDF_Creator::WriteDoc_Stage4() { } RetainPtr current_info = document_->GetInfo(); + const uint32_t current_info_objnum = + current_info ? current_info->GetObjNum() : 0; + const uint32_t parser_info_objnum = parser_ ? parser_->GetInfoObjNum() : 0; + const bool should_write_current_info = + current_info_objnum != 0 && current_info_objnum != parser_info_objnum; if (parser_) { CPDF_DictionaryLocker locker(parser_->GetCombinedTrailer()); for (const auto& it : locker) { @@ -553,7 +558,7 @@ CPDF_Creator::Stage CPDF_Creator::WriteDoc_Stage4() { if (key == "Encrypt" || key == "Size" || key == "Filter" || key == "Index" || key == "Length" || key == "Prev" || key == "W" || key == "XRefStm" || key == "ID" || key == "DecodeParms" || - key == "Type" || (key == "Info" && current_info)) { + key == "Type" || (key == "Info" && should_write_current_info)) { continue; } if (!archive_->WriteString(("/")) || @@ -571,9 +576,9 @@ CPDF_Creator::Stage CPDF_Creator::WriteDoc_Stage4() { return Stage::kInvalid; } } - if (current_info && current_info->GetObjNum() != 0) { + if (should_write_current_info) { if (!archive_->WriteString("/Info ") || - !archive_->WriteDWord(current_info->GetObjNum()) || + !archive_->WriteDWord(current_info_objnum) || !archive_->WriteString(" 0 R\r\n")) { return Stage::kInvalid; } diff --git a/core/fpdfapi/font/cpdf_fontglobals.cpp b/core/fpdfapi/font/cpdf_fontglobals.cpp index bd57de4cfd..17f3b48c02 100644 --- a/core/fpdfapi/font/cpdf_fontglobals.cpp +++ b/core/fpdfapi/font/cpdf_fontglobals.cpp @@ -18,10 +18,13 @@ #include "core/fpdfapi/parser/cpdf_document.h" #include "core/fxcrt/check.h" #include "core/fxcrt/containers/contains.h" +#include "core/fxcrt/epdf_tls.h" namespace { -CPDF_FontGlobals* g_FontGlobals = nullptr; +// EmbedPDF: thread-confined runtime - each worker thread owns its own font +// globals (stock fonts + predefined CMaps), created/destroyed on that thread. +EPDF_TLS CPDF_FontGlobals* g_FontGlobals = nullptr; RetainPtr LoadPredefinedCMap(ByteStringView name) { if (!name.IsEmpty() && name[0] == '/') { diff --git a/core/fpdfapi/page/cpdf_colorspace.cpp b/core/fpdfapi/page/cpdf_colorspace.cpp index e49d7af436..186fe40b55 100644 --- a/core/fpdfapi/page/cpdf_colorspace.cpp +++ b/core/fpdfapi/page/cpdf_colorspace.cpp @@ -38,6 +38,7 @@ #include "core/fxcrt/check_op.h" #include "core/fxcrt/compiler_specific.h" #include "core/fxcrt/containers/contains.h" +#include "core/fxcrt/epdf_tls.h" #include "core/fxcrt/data_vector.h" #include "core/fxcrt/fx_2d_size.h" #include "core/fxcrt/fx_safe_types.h" @@ -450,7 +451,9 @@ class StockColorSpaces { RetainPtr pattern_; }; -StockColorSpaces* g_stock_colorspaces = nullptr; +// EmbedPDF: thread-confined runtime - per-thread stock device colorspaces +// (gray/rgb/cmyk/pattern), created/destroyed on the owning thread. +EPDF_TLS StockColorSpaces* g_stock_colorspaces = nullptr; } // namespace diff --git a/core/fpdfapi/page/cpdf_streamcontentparser.cpp b/core/fpdfapi/page/cpdf_streamcontentparser.cpp index 64f366c2fc..b22e4c8d7c 100644 --- a/core/fpdfapi/page/cpdf_streamcontentparser.cpp +++ b/core/fpdfapi/page/cpdf_streamcontentparser.cpp @@ -42,6 +42,7 @@ #include "core/fxcrt/check.h" #include "core/fxcrt/compiler_specific.h" #include "core/fxcrt/containers/contains.h" +#include "core/fxcrt/epdf_tls.h" #include "core/fxcrt/fx_safe_types.h" #include "core/fxcrt/scoped_set_insertion.h" #include "core/fxcrt/span.h" @@ -68,7 +69,9 @@ const char kPathOperatorClosePath = 'h'; const char kPathOperatorRectangle[] = "re"; using OpCodes = std::map; -OpCodes* g_opcodes = nullptr; +// EmbedPDF: thread-confined runtime - per-thread content-operator dispatch +// table, lazily built and torn down on the owning thread. +EPDF_TLS OpCodes* g_opcodes = nullptr; CFX_FloatRect GetShadingBBox(CPDF_ShadingPattern* pShading, const CFX_Matrix& matrix) { diff --git a/core/fpdfapi/render/cpdf_renderstatus.cpp b/core/fpdfapi/render/cpdf_renderstatus.cpp index 8494f1c615..8485bc5db4 100644 --- a/core/fpdfapi/render/cpdf_renderstatus.cpp +++ b/core/fpdfapi/render/cpdf_renderstatus.cpp @@ -55,6 +55,7 @@ #include "core/fxcrt/compiler_specific.h" #include "core/fxcrt/containers/contains.h" #include "core/fxcrt/data_vector.h" +#include "core/fxcrt/epdf_tls.h" #include "core/fxcrt/fx_2d_size.h" #include "core/fxcrt/fx_safe_types.h" #include "core/fxcrt/fx_system.h" @@ -80,7 +81,9 @@ namespace { constexpr int kRenderMaxRecursionDepth = 64; -int g_CurrentRecursionDepth = 0; +// EmbedPDF: thread-confined runtime - per-thread render recursion counter so +// concurrent renders on different workers don't corrupt each other's depth. +EPDF_TLS int g_CurrentRecursionDepth = 0; CFX_FillRenderOptions GetFillOptionsForDrawPathWithBlend( const CPDF_RenderOptions::Options& options, diff --git a/core/fxcodec/icc/icc_transform.cpp b/core/fxcodec/icc/icc_transform.cpp index 0cd02e7b2f..fad316a265 100644 --- a/core/fxcodec/icc/icc_transform.cpp +++ b/core/fxcodec/icc/icc_transform.cpp @@ -20,6 +20,22 @@ namespace fxcodec { namespace { +// EmbedPDF: thread-confined runtime - LCMS verification gate. +// +// The cms* calls below (cmsOpenProfileFromMem, cmsCreate_sRGBProfile, +// cmsCreateTransform, cmsDoTransform, ...) use the default/null cmsContext, +// which is a process-global in lcms2. We intentionally do NOT rewrite this for +// the first thread_local slice: per-profile/per-transform work on the default +// context is independent across threads in practice, and our workers create +// and use their own profiles/transforms. +// +// Gate (not a code change): run the threaded soak (testing/tools:epdf_thread_ +// soak) under ThreadSanitizer with ICC-heavy PDFs before lifting the server +// pool cap. If TSAN flags contention/races in the default context, move to a +// per-thread cmsContext via cmsCreateContext + the cms*THR APIs (or guard +// transform creation with a mutex). Do not enable ICC-heavy concurrency in +// production until this gate is green. + // For use with std::unique_ptr. struct CmsProfileDeleter { inline void operator()(cmsHPROFILE p) { cmsCloseProfile(p); } diff --git a/core/fxcrt/cfx_timer.cpp b/core/fxcrt/cfx_timer.cpp index 658f39a373..48fa5a07da 100644 --- a/core/fxcrt/cfx_timer.cpp +++ b/core/fxcrt/cfx_timer.cpp @@ -9,11 +9,16 @@ #include #include "core/fxcrt/check.h" +#include "core/fxcrt/epdf_tls.h" namespace { using TimerMap = std::map; -TimerMap* g_pwl_timer_map = nullptr; +// EmbedPDF: thread-confined runtime - per-thread timer map. The PWL timer +// subsystem is inactive in headless server rendering, but the global is still +// made per-thread so per-thread InitializeGlobals()/DestroyGlobals() (and the +// CHECK(!g_pwl_timer_map) inside Init) hold independently on each worker. +EPDF_TLS TimerMap* g_pwl_timer_map = nullptr; } // namespace diff --git a/core/fxcrt/epdf_tls.h b/core/fxcrt/epdf_tls.h new file mode 100644 index 0000000000..bca3c6eba3 --- /dev/null +++ b/core/fxcrt/epdf_tls.h @@ -0,0 +1,30 @@ +// Copyright 2025 The EmbedPDF Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// EmbedPDF: thread-confined runtime support. +// +// EPDF_TLS expands to `thread_local` when the build is configured with the +// `embedpdf_thread_local_globals` GN arg (which defines +// EPDF_THREAD_LOCAL_GLOBALS), and to nothing otherwise. It is used to give +// each worker thread its own copy of PDFium's process-global singletons so +// that N threads can run shared-nothing in a single process. +// +// Contract: a thread that touches any EPDF_TLS-backed global must initialize +// PDFium on that thread (EPDF_InitThread), use only handles created on that +// thread, and tear down on the same thread (EPDF_ShutdownThread). PDFium +// handles must never cross threads. +// +// Default (flag off) keeps every global as an ordinary process-global, so +// targets that do not opt in and upstream rebases are byte-for-byte unchanged. + +#ifndef CORE_FXCRT_EPDF_TLS_H_ +#define CORE_FXCRT_EPDF_TLS_H_ + +#if defined(EPDF_THREAD_LOCAL_GLOBALS) +#define EPDF_TLS thread_local +#else +#define EPDF_TLS +#endif + +#endif // CORE_FXCRT_EPDF_TLS_H_ diff --git a/core/fxcrt/fx_extension.cpp b/core/fxcrt/fx_extension.cpp index 28b4825b40..91c2151a94 100644 --- a/core/fxcrt/fx_extension.cpp +++ b/core/fxcrt/fx_extension.cpp @@ -6,6 +6,7 @@ #include "core/fxcrt/fx_extension.h" +#include #include #include @@ -24,7 +25,20 @@ time_t DefaultTimeFunction() { } struct tm* DefaultLocaltimeFunction(const time_t* tp) { - return localtime(tp); + // EmbedPDF: thread-confined runtime. Plain localtime() returns a pointer to + // shared static storage, which is a data race across worker threads. Fill a + // thread_local tm via the reentrant localtime_r/localtime_s instead so each + // thread gets its own result. struct tm is trivially destructible, so the + // thread_local adds no per-thread teardown cost. + thread_local struct tm result; +#if defined(_WIN32) + if (localtime_s(&result, tp) != 0) { + return nullptr; + } + return &result; +#else + return localtime_r(tp, &result); +#endif } time_t (*g_time_func)() = DefaultTimeFunction; diff --git a/core/fxcrt/fx_random.cpp b/core/fxcrt/fx_random.cpp index c0733698c9..ea0b63e447 100644 --- a/core/fxcrt/fx_random.cpp +++ b/core/fxcrt/fx_random.cpp @@ -9,6 +9,7 @@ #include #include "build/build_config.h" +#include "core/fxcrt/epdf_tls.h" #include "core/fxcrt/fx_memory.h" #include "core/fxcrt/fx_string.h" #include "core/fxcrt/fx_system.h" @@ -33,8 +34,10 @@ struct MTContext { std::array mt; }; -bool g_bHaveGlobalSeed = false; -uint32_t g_nGlobalSeed = 0; +// EmbedPDF: thread-confined runtime - per-thread RNG seed state so concurrent +// workers don't race on the lazy global-seed initialization. +EPDF_TLS bool g_bHaveGlobalSeed = false; +EPDF_TLS uint32_t g_nGlobalSeed = 0; #if BUILDFLAG(IS_WIN) bool GenerateSeedFromCryptoRandom(uint32_t* pSeed) { diff --git a/core/fxcrt/fx_system.cpp b/core/fxcrt/fx_system.cpp index d5c00af4df..ff3a72d38f 100644 --- a/core/fxcrt/fx_system.cpp +++ b/core/fxcrt/fx_system.cpp @@ -12,12 +12,15 @@ #include "build/build_config.h" #include "core/fxcrt/compiler_specific.h" +#include "core/fxcrt/epdf_tls.h" #include "core/fxcrt/fx_extension.h" namespace { #if !BUILDFLAG(IS_WIN) -uint32_t g_last_error = 0; +// EmbedPDF: thread-confined runtime - per-thread last-error, written during +// parse/render and read back by FPDF_GetLastError on the same thread. +EPDF_TLS uint32_t g_last_error = 0; #endif template diff --git a/core/fxge/cfx_gemodule.cpp b/core/fxge/cfx_gemodule.cpp index e67b86ceca..b4294e8085 100644 --- a/core/fxge/cfx_gemodule.cpp +++ b/core/fxge/cfx_gemodule.cpp @@ -7,12 +7,16 @@ #include "core/fxge/cfx_gemodule.h" #include "core/fxcrt/check.h" +#include "core/fxcrt/epdf_tls.h" #include "core/fxge/cfx_folderfontinfo.h" #include "core/fxge/cfx_fontmgr.h" namespace { -CFX_GEModule* g_pGEModule = nullptr; +// EmbedPDF: thread-confined runtime - each worker thread owns its own +// CFX_GEModule (FreeType FT_Library + font/glyph caches), created and destroyed +// on that thread. +EPDF_TLS CFX_GEModule* g_pGEModule = nullptr; } // namespace diff --git a/fpdfsdk/BUILD.gn b/fpdfsdk/BUILD.gn index 6132162e33..6c8aa9f80a 100644 --- a/fpdfsdk/BUILD.gn +++ b/fpdfsdk/BUILD.gn @@ -15,6 +15,7 @@ source_set("fpdfsdk") { "epdf_png_shim.cpp", "epdf_jpeg_shim.cpp", "epdf_redact.cpp", + "epdf_threading.cpp", "cpdfsdk_annot.cpp", "cpdfsdk_annot.h", "cpdfsdk_annotiteration.cpp", diff --git a/fpdfsdk/cpdfsdk_helpers.cpp b/fpdfsdk/cpdfsdk_helpers.cpp index 766f6033a5..9bab8ec218 100644 --- a/fpdfsdk/cpdfsdk_helpers.cpp +++ b/fpdfsdk/cpdfsdk_helpers.cpp @@ -38,6 +38,18 @@ namespace { constexpr char kQuadPoints[] = "QuadPoints"; +// EmbedPDF: thread-confined runtime - config callbacks, NOT runtime knobs. +// These two process-globals are intentionally left process-wide (not EPDF_TLS): +// they are immutable configuration that MUST be set once during process +// bootstrap, before any worker thread starts, and MUST NOT be mutated while +// workers are live. Setting them per-request/per-thread is unsupported. The +// server sets neither after startup, so there is no cross-thread write race. +// +// (Other inactive process-globals in our build config - the Skia font manager +// and the Windows print-mode globals - are compiled out of the headless server +// runtime. They would be unsafe under multi-threading if those subsystems were +// ever re-enabled, and would need the same treatment then.) + // 0 bit: FPDF_POLICY_MACHINETIME_ACCESS uint32_t g_sandbox_policy = 0xFFFFFFFF; diff --git a/fpdfsdk/epdf_threading.cpp b/fpdfsdk/epdf_threading.cpp new file mode 100644 index 0000000000..91fff70f02 --- /dev/null +++ b/fpdfsdk/epdf_threading.cpp @@ -0,0 +1,30 @@ +// Copyright 2026 The EmbedPDF Authors +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// EmbedPDF: thread-confined runtime lifecycle. +// +// EPDF_InitThread / EPDF_ShutdownThread make the per-thread PDFium lifecycle +// explicit for callers that run PDFium on worker threads. With +// embedpdf_thread_local_globals enabled (see core/fxcrt/epdf_tls.h) each worker +// thread owns its own PDFium globals, so every such thread must initialize and +// tear down PDFium itself. With the flag disabled these are exact aliases of +// FPDF_InitLibrary / FPDF_DestroyLibrary. +// +// These are intentionally exported for ALL targets (including wasm, where they +// simply wrap the normal init/destroy) so shared runtime code can call a single +// lifecycle entry point regardless of platform. + +#include "public/fpdfview.h" + +FPDF_EXPORT void FPDF_CALLCONV EPDF_InitThread() { + // Routes through FPDF_InitLibrary so the standard config/init path runs for + // the calling thread. + FPDF_InitLibrary(); +} + +FPDF_EXPORT void FPDF_CALLCONV EPDF_ShutdownThread() { + // Lifecycle-strict: callers must have already closed every PDFium handle + // created on this thread before invoking this. + FPDF_DestroyLibrary(); +} diff --git a/fpdfsdk/fpdf_view.cpp b/fpdfsdk/fpdf_view.cpp index 04686a6b7c..c39d414e04 100644 --- a/fpdfsdk/fpdf_view.cpp +++ b/fpdfsdk/fpdf_view.cpp @@ -44,6 +44,7 @@ #include "core/fxcrt/cfx_timer.h" #include "core/fxcrt/check_op.h" #include "core/fxcrt/compiler_specific.h" +#include "core/fxcrt/epdf_tls.h" #include "core/fxcrt/fx_extension.h" #include "core/fxcrt/fx_memcpy_wrappers.h" #include "core/fxcrt/fx_safe_types.h" @@ -116,7 +117,9 @@ static_assert(static_cast(CFX_DefaultRenderDevice::RendererType::kSkia) == namespace { -bool g_bLibraryInitialized = false; +// EmbedPDF: thread-confined runtime - each worker thread tracks its own +// library-initialized state so per-thread Init/Destroy don't race. +EPDF_TLS bool g_bLibraryInitialized = false; void SetRendererType(FPDF_RENDERER_TYPE public_type) { // Internal definition of renderer types must stay updated with respect to diff --git a/fpdfsdk/fpdf_view_c_api_test.c b/fpdfsdk/fpdf_view_c_api_test.c index 9ccefbba11..fc3678e32e 100644 --- a/fpdfsdk/fpdf_view_c_api_test.c +++ b/fpdfsdk/fpdf_view_c_api_test.c @@ -536,6 +536,8 @@ int CheckPDFiumCApi() { CHK(FPDF_GetXFAPacketName); CHK(FPDF_InitLibrary); CHK(FPDF_InitLibraryWithConfig); + CHK(EPDF_InitThread); + CHK(EPDF_ShutdownThread); CHK(EPDF_GetPageBoxByIndex); CHK(EPDF_GetPageUserUnitByIndex); CHK(EPDF_LoadBaseDocument); diff --git a/pdfium.gni b/pdfium.gni index 73ea9f46bd..6c681945de 100644 --- a/pdfium.gni +++ b/pdfium.gni @@ -91,6 +91,14 @@ declare_args() { # Don't build against bundled zlib. use_system_zlib = false + + # EmbedPDF: thread-confined runtime. When true, PDFium's process-global + # singletons become per-thread (thread_local via EPDF_TLS), so each worker + # thread owns its own PDFium state and N threads can render shared-nothing in + # one process. Default off keeps globals process-wide; enable only for native + # server targets that follow the per-thread init/shutdown contract. Never + # makes arbitrary PDFium usage thread-safe; handles must not cross threads. + embedpdf_thread_local_globals = false } assert(!pdf_is_complete_lib || !is_component_build, diff --git a/public/fpdf_ext.h b/public/fpdf_ext.h index 068a977c12..877788bfbe 100644 --- a/public/fpdf_ext.h +++ b/public/fpdf_ext.h @@ -84,7 +84,10 @@ FPDF_EXPORT void FPDF_CALLCONV FSDK_SetTimeFunction(time_t (*func)()); // behave poorly in production environments. // // func - Function pointer to alternate implementation of localtime(), or -// NULL to restore to actual localtime() call itself. +// NULL to restore the thread-safe default localtime wrapper (which +// fills a thread-local tm via localtime_r/localtime_s). EmbedPDF: the +// default is no longer plain localtime(), which returned shared static +// storage and raced across threads. FPDF_EXPORT void FPDF_CALLCONV FSDK_SetLocaltimeFunction(struct tm* (*func)(const time_t*)); diff --git a/public/fpdfview.h b/public/fpdfview.h index 8a2ddd2671..66f8ebe992 100644 --- a/public/fpdfview.h +++ b/public/fpdfview.h @@ -320,6 +320,24 @@ FPDF_EXPORT void FPDF_CALLCONV FPDF_InitLibrary(); // closing the library with this function. FPDF_EXPORT void FPDF_CALLCONV FPDF_DestroyLibrary(); +// Experimental EmbedPDF Extension API. +// Function: EPDF_InitThread / EPDF_ShutdownThread +// Thread-confined runtime lifecycle. Initialize / tear down PDFium for +// the CALLING thread. +// Comments: +// When the runtime is built with per-thread globals +// (embedpdf_thread_local_globals), each worker thread owns its own +// PDFium state, so every thread that uses PDFium MUST call +// EPDF_InitThread before creating any handle and EPDF_ShutdownThread +// after closing every handle it created. Handles must never cross +// threads. When per-thread globals are disabled, these are exact +// aliases of FPDF_InitLibrary / FPDF_DestroyLibrary and are safe to +// call once on a single thread. EPDF_ShutdownThread is lifecycle- +// strict: only legal after all handles on the calling thread are +// closed. +FPDF_EXPORT void FPDF_CALLCONV EPDF_InitThread(); +FPDF_EXPORT void FPDF_CALLCONV EPDF_ShutdownThread(); + // Policy for accessing the local machine time. #define FPDF_POLICY_MACHINETIME_ACCESS 0 diff --git a/scripts/embedpdf-runtime/build-target.sh b/scripts/embedpdf-runtime/build-target.sh index 0326635369..84b4662113 100755 --- a/scripts/embedpdf-runtime/build-target.sh +++ b/scripts/embedpdf-runtime/build-target.sh @@ -4,6 +4,10 @@ set -euo pipefail SOURCE_DIR="${PDF_RUNTIME_SOURCE_DIR:-$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)}" TARGET="${1:-}" PDF_IS_COMPLETE_LIB=true +# EmbedPDF: thread-confined runtime. Off by default; enabled per-target below +# for the native server builds. wasm isolates globals per-instance, so it stays +# off there. +EMBEDPDF_TLS_GLOBALS=false if [[ -z "$TARGET" ]]; then echo "usage: $0 " >&2 @@ -63,6 +67,13 @@ case "$TARGET" in ;; esac +# EmbedPDF: enable per-thread PDFium globals for the native server targets so +# the server worker pool can render in parallel in-process. wasm stays off +# (each instance already isolates globals via its own linear memory). +if [[ "$TARGET" != "wasm32" ]]; then + EMBEDPDF_TLS_GLOBALS=true +fi + PDF_RUNTIME_TARGET_OS_LIST="${PDF_RUNTIME_TARGET_OS_LIST:-$GN_TARGET_OS}" \ "$SOURCE_DIR/scripts/embedpdf-runtime/ensure-deps.sh" @@ -91,6 +102,7 @@ pdf_is_standalone=true use_debug_fission=false pdf_is_complete_lib=$PDF_IS_COMPLETE_LIB pdf_use_partition_alloc=false +embedpdf_thread_local_globals=$EMBEDPDF_TLS_GLOBALS symbol_level=0 target_os="$GN_TARGET_OS" target_cpu="$GN_TARGET_CPU"${EXTRA_ARGS:-} diff --git a/scripts/embedpdf-runtime/test-target.sh b/scripts/embedpdf-runtime/test-target.sh index 07c25687b0..0c1321cbbc 100755 --- a/scripts/embedpdf-runtime/test-target.sh +++ b/scripts/embedpdf-runtime/test-target.sh @@ -4,6 +4,11 @@ set -euo pipefail SOURCE_DIR="${PDF_RUNTIME_SOURCE_DIR:-$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)}" TARGET="${1:-}" TEST_SUITE="${PDFIUM_TEST_SUITE:-all}" +# EmbedPDF: thread-confined runtime. test-target.sh only builds native host +# targets, which is exactly where we ship the flag on, so default it on here to +# exercise the same variant we ship. Override with EMBEDPDF_TLS_GLOBALS=false to +# build the baseline (process-global) variant for comparison/regression. +EMBEDPDF_TLS_GLOBALS="${EMBEDPDF_TLS_GLOBALS:-true}" if [[ -z "$TARGET" ]]; then echo "usage: $0 " >&2 @@ -71,6 +76,7 @@ pdf_is_standalone=true use_debug_fission=false pdf_is_complete_lib=false pdf_use_partition_alloc=false +embedpdf_thread_local_globals=$EMBEDPDF_TLS_GLOBALS symbol_level=1 target_os="$GN_TARGET_OS" target_cpu="$GN_TARGET_CPU"${EXTRA_ARGS:-} diff --git a/scripts/embedpdf-runtime/thread-soak-target.sh b/scripts/embedpdf-runtime/thread-soak-target.sh new file mode 100755 index 0000000000..b2fe0b2b43 --- /dev/null +++ b/scripts/embedpdf-runtime/thread-soak-target.sh @@ -0,0 +1,101 @@ +#!/usr/bin/env bash +set -euo pipefail + +# EmbedPDF: build and run the thread-confined runtime soak harness +# (testing/tools:epdf_thread_soak). This is the gate for the thread_local +# globals work: run it (ideally under TSAN) before lifting the server pool cap. +# +# Usage: +# thread-soak-target.sh [-- ] +# +# Env: +# EMBEDPDF_TLS_GLOBALS default "true". Set "false" to build the baseline +# (process-global) variant for an A/B comparison. +# EMBEDPDF_TSAN default "0". Set "1" to build with is_tsan=true. + +SOURCE_DIR="${PDF_RUNTIME_SOURCE_DIR:-$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd)}" +TARGET="${1:-}" +PDF_PATH="${2:-}" +EMBEDPDF_TLS_GLOBALS="${EMBEDPDF_TLS_GLOBALS:-true}" +EMBEDPDF_TSAN="${EMBEDPDF_TSAN:-0}" + +if [[ -z "$TARGET" || -z "$PDF_PATH" ]]; then + echo "usage: $0 [-- ]" >&2 + exit 1 +fi +shift 2 || true +if [[ "${1:-}" == "--" ]]; then + shift +fi + +case "$TARGET" in + darwin-arm64) + GN_TARGET_OS="mac" + GN_TARGET_CPU="arm64" + ;; + darwin-x64) + GN_TARGET_OS="mac" + GN_TARGET_CPU="x64" + ;; + linux-x64) + GN_TARGET_OS="linux" + GN_TARGET_CPU="x64" + ;; + linux-arm64) + GN_TARGET_OS="linux" + GN_TARGET_CPU="arm64" + EXTRA_ARGS=$'\narm_control_flow_integrity="none"' + ;; + *) + echo "thread-soak-target.sh only supports host-native targets: darwin-arm64, darwin-x64, linux-x64, linux-arm64" >&2 + exit 1 + ;; +esac + +TSAN_ARG="" +if [[ "$EMBEDPDF_TSAN" == "1" ]]; then + TSAN_ARG=$'\nis_tsan=true' +fi + +PDF_RUNTIME_TARGET_OS_LIST="${PDF_RUNTIME_TARGET_OS_LIST:-$GN_TARGET_OS}" \ + "$SOURCE_DIR/scripts/embedpdf-runtime/ensure-deps.sh" + +"$SOURCE_DIR/scripts/embedpdf-runtime/apply-patches.sh" "$TARGET" + +if [[ "$TARGET" == linux-* ]]; then + ( + cd "$SOURCE_DIR" + build/install-build-deps.sh --no-prompt + build/linux/sysroot_scripts/install-sysroot.py "--arch=$GN_TARGET_CPU" + ) +fi + +OUT="$SOURCE_DIR/out/embedpdf-runtime-thread-soak-$TARGET" +mkdir -p "$OUT" + +cat > "$OUT/args.gn" < load -> render page 0 -> (optional) encrypted save +// -> close -> FPDF_DestroyLibrary. Each worker only ever touches handles it +// created, matching the thread-confined contract. +// +// With embedpdf_thread_local_globals OFF this exercises (and is expected to +// trip) the shared process-global init/render races. With the flag ON every +// thread owns its own PDFium state and the soak should pass cleanly. The +// encrypted-save path is included on purpose: pending security is now stored on +// CPDF_Document, so it must survive concurrent SetEncryption -> save across +// threads. Intended to be run under ThreadSanitizer as the gate before the +// server worker-pool cap is lifted. + +#include "testing/tools/epdf_layer_tool_common.h" + +#include + +#include +#include +#include +#include +#include +#include + +#include "public/cpp/fpdf_scopers.h" +#include "public/fpdf_save.h" +#include "public/fpdfview.h" + +namespace { + +// Fixed render surface keeps per-thread memory bounded while still exercising +// the rasterizer, font cache, and stock colorspaces. +constexpr int kRenderWidth = 300; +constexpr int kRenderHeight = 400; + +size_t ParseSizeArg(const std::string& arg, + const char* prefix, + size_t fallback) { + const std::string prefix_string(prefix); + if (arg.rfind(prefix_string, 0) != 0) { + return fallback; + } + return static_cast( + std::strtoull(arg.substr(prefix_string.size()).c_str(), nullptr, 10)); +} + +struct Options { + size_t threads = 4; + size_t iterations = 50; + bool render = true; + bool encrypt = true; +}; + +const char* kUsageExtra = + "[--threads=4] [--iterations=50] [--no-render] [--no-encrypt]"; + +bool RenderFirstPage(FPDF_DOCUMENT doc) { + ScopedFPDFPage page(FPDF_LoadPage(doc, 0)); + if (!page) { + return false; + } + ScopedFPDFBitmap bitmap(FPDFBitmap_Create(kRenderWidth, kRenderHeight, 0)); + if (!bitmap) { + return false; + } + FPDFBitmap_FillRect(bitmap.get(), 0, 0, kRenderWidth, kRenderHeight, + 0xFFFFFFFF); + FPDF_RenderPageBitmap(bitmap.get(), page.get(), 0, 0, kRenderWidth, + kRenderHeight, 0, FPDF_ANNOT); + // Touch the buffer so the render isn't optimized away. + return FPDFBitmap_GetBuffer(bitmap.get()) != nullptr; +} + +bool EncryptAndSave(FPDF_DOCUMENT doc) { + if (!EPDF_SetEncryption(doc, "user", "owner", + EPDF_PERM_PRINT | EPDF_PERM_COPY)) { + return false; + } + unsigned long out_size = 0; + void* buffer = EPDF_SaveDocumentToOwnedBuffer(doc, 0, &out_size); + const bool ok = buffer != nullptr && out_size > 0; + EPDF_FreeBuffer(buffer); + return ok; +} + +bool RunWorker(const std::vector& bytes, + const Options& opts, + size_t worker_index) { + for (size_t i = 0; i < opts.iterations; ++i) { + FPDF_InitLibrary(); + { + ScopedFPDFDocument doc(FPDF_LoadMemDocument64( + bytes.data(), bytes.size(), nullptr)); + if (!doc) { + std::fprintf(stderr, "worker %zu iter %zu: load failed\n", worker_index, + i); + FPDF_DestroyLibrary(); + return false; + } + if (opts.render && !RenderFirstPage(doc.get())) { + std::fprintf(stderr, "worker %zu iter %zu: render failed\n", + worker_index, i); + FPDF_DestroyLibrary(); + return false; + } + if (opts.encrypt && !EncryptAndSave(doc.get())) { + std::fprintf(stderr, "worker %zu iter %zu: encrypt-save failed\n", + worker_index, i); + FPDF_DestroyLibrary(); + return false; + } + } // Document is closed here, before tearing down the library. + FPDF_DestroyLibrary(); + } + return true; +} + +} // namespace + +int main(int argc, char** argv) { + if (argc < 2) { + epdf_layer_tool::PrintUsage(argv[0], kUsageExtra); + return 2; + } + + std::string path = argv[1]; + Options opts; + for (int i = 2; i < argc; ++i) { + const std::string arg = argv[i]; + if (arg.rfind("--threads=", 0) == 0) { + opts.threads = ParseSizeArg(arg, "--threads=", opts.threads); + } else if (arg.rfind("--iterations=", 0) == 0) { + opts.iterations = ParseSizeArg(arg, "--iterations=", opts.iterations); + } else if (arg == "--no-render") { + opts.render = false; + } else if (arg == "--no-encrypt") { + opts.encrypt = false; + } else { + epdf_layer_tool::PrintUsage(argv[0], kUsageExtra); + return 2; + } + } + + if (opts.threads == 0 || opts.iterations == 0) { + std::fprintf(stderr, "Thread count and iterations must be positive.\n"); + return 2; + } + + std::vector bytes; + if (!epdf_layer_tool::ReadFile(path, &bytes)) { + std::fprintf(stderr, "Failed to read %s\n", path.c_str()); + return 1; + } + + std::atomic failures{0}; + std::vector workers; + workers.reserve(opts.threads); + for (size_t t = 0; t < opts.threads; ++t) { + workers.emplace_back([&bytes, &opts, &failures, t]() { + if (!RunWorker(bytes, opts, t)) { + failures.fetch_add(1, std::memory_order_relaxed); + } + }); + } + for (auto& worker : workers) { + worker.join(); + } + + const size_t failed = failures.load(std::memory_order_relaxed); + if (failed != 0) { + std::fprintf(stderr, "thread soak FAILED: %zu/%zu workers errored\n", failed, + opts.threads); + return 1; + } + + std::printf( + "thread soak OK: threads=%zu iterations=%zu render=%d encrypt=%d\n", + opts.threads, opts.iterations, opts.render ? 1 : 0, + opts.encrypt ? 1 : 0); + return 0; +} From af64b09eb1b9ef30fbe227e5488eeb71f4988a5d Mon Sep 17 00:00:00 2001 From: Bob Singor Date: Tue, 2 Jun 2026 23:39:06 +0300 Subject: [PATCH 2/7] Make s_CurrentRecursionDepth thread-local Mark the parser's static recursion depth variable as thread-local (EPDF_TLS) in both the header and source. Adds an #include for core/fxcrt/epdf_tls.h. This ensures each thread has its own s_CurrentRecursionDepth to avoid cross-thread interference when parsing concurrently; kParserMaxRecursionDepth is unchanged. --- core/fpdfapi/parser/cpdf_syntax_parser.cpp | 2 +- core/fpdfapi/parser/cpdf_syntax_parser.h | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/core/fpdfapi/parser/cpdf_syntax_parser.cpp b/core/fpdfapi/parser/cpdf_syntax_parser.cpp index f765a10ead..3753019c64 100644 --- a/core/fpdfapi/parser/cpdf_syntax_parser.cpp +++ b/core/fpdfapi/parser/cpdf_syntax_parser.cpp @@ -76,7 +76,7 @@ class ReadableSubStream final : public IFX_SeekableReadStream { } // namespace // static -int CPDF_SyntaxParser::s_CurrentRecursionDepth = 0; +EPDF_TLS int CPDF_SyntaxParser::s_CurrentRecursionDepth = 0; // static std::unique_ptr CPDF_SyntaxParser::CreateForTesting( diff --git a/core/fpdfapi/parser/cpdf_syntax_parser.h b/core/fpdfapi/parser/cpdf_syntax_parser.h index a3c27a9342..283dcbb20c 100644 --- a/core/fpdfapi/parser/cpdf_syntax_parser.h +++ b/core/fpdfapi/parser/cpdf_syntax_parser.h @@ -15,6 +15,7 @@ #include "core/fpdfapi/parser/cpdf_stream.h" #include "core/fxcrt/data_vector.h" +#include "core/fxcrt/epdf_tls.h" #include "core/fxcrt/fx_types.h" #include "core/fxcrt/retain_ptr.h" #include "core/fxcrt/span.h" @@ -96,7 +97,7 @@ class CPDF_SyntaxParser { friend class cpdf_syntax_parser_ReadHexString_Test; static constexpr int kParserMaxRecursionDepth = 64; - static int s_CurrentRecursionDepth; + static EPDF_TLS int s_CurrentRecursionDepth; bool ReadBlockAt(FX_FILESIZE read_pos); bool GetCharAtBackward(FX_FILESIZE pos, uint8_t* ch); From 0267271acea5ab405de87b409e12a518cbd5d3d5 Mon Sep 17 00:00:00 2001 From: Bob Singor Date: Tue, 2 Jun 2026 23:53:18 +0300 Subject: [PATCH 3/7] Add test PDFs with bad ICC profiles Add two PDF fixtures under testing/resources/pixel to exercise ICC profile parsing: icc_profile_bad_component.pdf embeds an ICCBased color space with N=1 (one component instead of the expected 3) but still triggers DetectSRGB(); icc_profile_bad_value.pdf contains a profile/value (0x80000000) that overflows when multiplied by 255. These files are used by pixel tests to validate robustness and error handling of ICC profile processing. --- .../resources/pixel/icc_profile_bad_component.pdf | Bin 0 -> 875 bytes testing/resources/pixel/icc_profile_bad_value.pdf | Bin 0 -> 906 bytes 2 files changed, 0 insertions(+), 0 deletions(-) create mode 100644 testing/resources/pixel/icc_profile_bad_component.pdf create mode 100644 testing/resources/pixel/icc_profile_bad_value.pdf diff --git a/testing/resources/pixel/icc_profile_bad_component.pdf b/testing/resources/pixel/icc_profile_bad_component.pdf new file mode 100644 index 0000000000000000000000000000000000000000..03ead69421ad4e99471a68cb5d6bbde10f10ff1d GIT binary patch literal 875 zcmZuwU2obj6n*!vxGzzu#z#V$6lsWQgwnz~T9Q6&LS1K^3&|E@W;;^bhy9j4>=#b^ z0kaJh#%3u|e00zKIQPW49Sj>@^PD(8e*OGSJaB=}zd*N309toXYlK!r6)pJE0Jn6B z652W_CcPfPGS$Jv+>c2%(oiB^l?pub`7Tc-%zVSXKcE}nfAmd3fttRlXtR|{13Jyw zCXL44W17=Ge}tKp6vnE~{S z_O+kYl-)Objpb5h;Gdguq!LJ1{?P-5|&+1*(KQrEo#LfX~<~E;LJo9AN&>{{6fnQ zuqUvB=%jKxHK)7J>7L4Me^7TD=dAMm=Z{~^1qbB(19UnJpn11hBQyhUcqW$?xaCXK z(4tKt>vkFD32kQf|Cm(nw3I*=xdGRHz7&ausb|^u2XqVkkG@T)km%ctWwhK7Kw-|d zVF-JTiQql?08@uB*D3FN%MMY?Ld8^asi)*;QblVX<0E9j$bo5d7z918F#-QEG5~2T zAA7Af?Y`Jom@iEVuIpGm+9>2J_KB4dW6dgXmkJG^c~)QzzNDDLs>qC3XM*y~H_-C^ z8YGem%%y>aFd%Z^P>9@kt(j@C3un8)rAx=`Kp`lE1)JC=>=N(VvCEnitJMZb6|9w9 zhzud+8cd3y#4ZT({6T2O@7 zZ{I|x-jj7u&pNf)^N%>Wc~({3>)FY&F&fp2ZaN;t@4uA;__y{4g$pvN&33-g3);T# z`LKXLm`hUZf_#s)FaE(?k50qUKF^^6IKq7Y05gh{)oOQuLP>44XDYNNMB*z_Y2~DF QWy)I5-KVTlxx5*$-^CI1MF0Q* literal 0 HcmV?d00001 From ad5c71a624196b31ffeaeb44c56c0f78c4fefaa6 Mon Sep 17 00:00:00 2001 From: Bob Singor Date: Wed, 3 Jun 2026 00:16:06 +0300 Subject: [PATCH 4/7] Fix gmtime handling and add platform defines Add platform-specific defines for gmtime variants in BUILD.gn (HAVE_GMTIME_R on Linux/ChromeOS/Android/mac/iOS, HAVE_GMTIME_S on Windows). Update cmsplugin.c::_cmsGetTime to guard against gmtime returning NULL: copy the result into ptr_time only if non-NULL and return a boolean success value, avoiding a NULL dereference inside the critical section. --- third_party/BUILD.gn | 6 ++++++ third_party/lcms/src/cmsplugin.c | 3 +++ 2 files changed, 9 insertions(+) diff --git a/third_party/BUILD.gn b/third_party/BUILD.gn index dfb74ec3ed..6384408d1e 100644 --- a/third_party/BUILD.gn +++ b/third_party/BUILD.gn @@ -306,6 +306,12 @@ source_set("fx_lcms2") { "lcms/src/cmswtpnt.c", "lcms/src/cmsxform.c", ] + if (is_linux || is_chromeos || is_android || is_mac || is_ios) { + defines = [ "HAVE_GMTIME_R" ] + } + if (is_win) { + defines = [ "HAVE_GMTIME_S" ] + } deps = [ "../core/fxcrt" ] } diff --git a/third_party/lcms/src/cmsplugin.c b/third_party/lcms/src/cmsplugin.c index 3876506dac..c5e33d937b 100644 --- a/third_party/lcms/src/cmsplugin.c +++ b/third_party/lcms/src/cmsplugin.c @@ -1057,7 +1057,10 @@ cmsBool _cmsGetTime(struct tm* ptr_time) _cmsEnterCriticalSectionPrimitive(&_cmsContextPoolHeadMutex); t = gmtime(&now); + if (t != NULL) + *ptr_time = *t; _cmsLeaveCriticalSectionPrimitive(&_cmsContextPoolHeadMutex); + return t != NULL; #endif if (t == NULL) From 2520dc17b58f42f93c43f872cc382f2914886ab8 Mon Sep 17 00:00:00 2001 From: Bob Singor Date: Wed, 3 Jun 2026 00:38:30 +0300 Subject: [PATCH 5/7] Update cmswtpnt.c --- third_party/lcms/src/cmswtpnt.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/third_party/lcms/src/cmswtpnt.c b/third_party/lcms/src/cmswtpnt.c index a73eaa7e36..a8648a33ae 100644 --- a/third_party/lcms/src/cmswtpnt.c +++ b/third_party/lcms/src/cmswtpnt.c @@ -30,16 +30,18 @@ // D50 - Widely used const cmsCIEXYZ* CMSEXPORT cmsD50_XYZ(void) { - static cmsCIEXYZ D50XYZ = {cmsD50X, cmsD50Y, cmsD50Z}; + static const cmsCIEXYZ D50XYZ = {cmsD50X, cmsD50Y, cmsD50Z}; return &D50XYZ; } const cmsCIExyY* CMSEXPORT cmsD50_xyY(void) { - static cmsCIExyY D50xyY; - - cmsXYZ2xyY(&D50xyY, cmsD50_XYZ()); + static const cmsCIExyY D50xyY = { + cmsD50X / (cmsD50X + cmsD50Y + cmsD50Z), + cmsD50Y / (cmsD50X + cmsD50Y + cmsD50Z), + cmsD50Y + }; return &D50xyY; } @@ -350,4 +352,3 @@ cmsBool CMSEXPORT cmsAdaptToIlluminant(cmsCIEXYZ* Result, return TRUE; } - From 74b6d9c2c5470891db37b384c48fd74ee8a70faf Mon Sep 17 00:00:00 2001 From: Bob Singor Date: Wed, 3 Jun 2026 00:40:46 +0300 Subject: [PATCH 6/7] Address thread-safety in lcms helpers Add clarifying comments about concurrency and ThreadSanitizer findings in two lcms sources. In cmsplugin.c note that gmtime()'s shared static result is copied while holding the LCMS mutex to avoid TSan-reported races when creating ICC profiles concurrently. In cmswtpnt.c document that the D50 XYZ constant is safe for concurrent readers and that the xyY value is computed in a thread-confined way to avoid writing to a shared static on each call. --- third_party/lcms/src/cmsplugin.c | 3 +++ third_party/lcms/src/cmswtpnt.c | 6 +++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/third_party/lcms/src/cmsplugin.c b/third_party/lcms/src/cmsplugin.c index c5e33d937b..ce7fb2eac7 100644 --- a/third_party/lcms/src/cmsplugin.c +++ b/third_party/lcms/src/cmsplugin.c @@ -1057,6 +1057,9 @@ cmsBool _cmsGetTime(struct tm* ptr_time) _cmsEnterCriticalSectionPrimitive(&_cmsContextPoolHeadMutex); t = gmtime(&now); + // EmbedPDF: copy gmtime()'s shared static result while still holding the + // LCMS mutex. ThreadSanitizer flags copying it after unlock when ICC + // profiles are created concurrently. if (t != NULL) *ptr_time = *t; _cmsLeaveCriticalSectionPrimitive(&_cmsContextPoolHeadMutex); diff --git a/third_party/lcms/src/cmswtpnt.c b/third_party/lcms/src/cmswtpnt.c index a8648a33ae..490035e9d5 100644 --- a/third_party/lcms/src/cmswtpnt.c +++ b/third_party/lcms/src/cmswtpnt.c @@ -30,6 +30,7 @@ // D50 - Widely used const cmsCIEXYZ* CMSEXPORT cmsD50_XYZ(void) { + // EmbedPDF: immutable shared D50 constant is safe for concurrent readers. static const cmsCIEXYZ D50XYZ = {cmsD50X, cmsD50Y, cmsD50Z}; return &D50XYZ; @@ -37,6 +38,10 @@ const cmsCIEXYZ* CMSEXPORT cmsD50_XYZ(void) const cmsCIExyY* CMSEXPORT cmsD50_xyY(void) { + // EmbedPDF: thread-confined runtime. + // This is equivalent to cmsXYZ2xyY(cmsD50_XYZ()), but avoids writing to a + // shared static on every call. ThreadSanitizer flags the old lazy + // recomputation when multiple documents create ICC transforms in parallel. static const cmsCIExyY D50xyY = { cmsD50X / (cmsD50X + cmsD50Y + cmsD50Z), cmsD50Y / (cmsD50X + cmsD50Y + cmsD50Z), @@ -351,4 +356,3 @@ cmsBool CMSEXPORT cmsAdaptToIlluminant(cmsCIEXYZ* Result, return TRUE; } - From deb36570d078a0489b5ab97d22e6a7c96815a126 Mon Sep 17 00:00:00 2001 From: Bob Singor Date: Mon, 15 Jun 2026 11:44:17 +0300 Subject: [PATCH 7/7] Add EPDFDoc page APIs (delete/set/normalize) Introduce new EmbedPDF extension APIs to operate on pages by PDF object number. LoadPageByValidatedIndex gains a `normalize` flag to override page rotation to 0 for normalized coordinates, and EPDFDoc_LoadPageByObjectNumberNormalized is exported. Add EPDFDoc_DeletePageByObjectNumber to delete the first visible page matching an indirect object number (with XFA-backed documents rejected), and EPDFDoc_SetPageRotationByObjectNumber to update a page's /Rotate value (validates rotate in 0..3). Update public/fpdfview.h with API docs, add tests and test registrations in fpdf_view_embeddertest.cpp and fpdf_view_c_api_test.c, and include fpdf_edit.h where needed. --- fpdfsdk/fpdf_view.cpp | 93 ++++++++++++++++++++++++++++-- fpdfsdk/fpdf_view_c_api_test.c | 2 + fpdfsdk/fpdf_view_embeddertest.cpp | 91 +++++++++++++++++++++++++++++ public/fpdfview.h | 54 +++++++++++++++++ 4 files changed, 236 insertions(+), 4 deletions(-) diff --git a/fpdfsdk/fpdf_view.cpp b/fpdfsdk/fpdf_view.cpp index c39d414e04..cf7ade5998 100644 --- a/fpdfsdk/fpdf_view.cpp +++ b/fpdfsdk/fpdf_view.cpp @@ -723,10 +723,14 @@ namespace { // Shared body of FPDF_LoadPage and EPDFDoc_LoadPageByObjectNumber. Validates // `page_index` against the document's page count, then constructs and returns -// a leaked page handle. Returns nullptr on any failure. +// a leaked page handle. When `normalize` is true, the page's rotation is +// overridden to 0 so all subsequent operations use normalized 0-degree +// coordinates (the intrinsic rotation is surfaced separately via +// EPDF_GetPageRotationByIndex). Returns nullptr on any failure. FPDF_PAGE LoadPageByValidatedIndex(FPDF_DOCUMENT document, CPDF_Document* doc, - int page_index) { + int page_index, + bool normalize) { if (page_index < 0 || page_index >= FPDF_GetPageCount(document)) { return nullptr; } @@ -750,6 +754,12 @@ FPDF_PAGE LoadPageByValidatedIndex(FPDF_DOCUMENT document, pPage->AddPageImageCache(); pPage->ParseContent(); + // Force rotation to 0 - this re-runs UpdateDimensions() so page_size_ and + // page_matrix_ are calculated as if rotation=0. + if (normalize) { + pPage->SetRotationOverride(0); + } + return FPDFPageFromIPDFPage(pPage.Leak()); } @@ -761,7 +771,8 @@ FPDF_EXPORT FPDF_PAGE FPDF_CALLCONV FPDF_LoadPage(FPDF_DOCUMENT document, if (!doc) { return nullptr; } - return LoadPageByValidatedIndex(document, doc, page_index); + return LoadPageByValidatedIndex(document, doc, page_index, + /*normalize=*/false); } FPDF_EXPORT FPDF_PAGE FPDF_CALLCONV @@ -770,7 +781,19 @@ EPDFDoc_LoadPageByObjectNumber(FPDF_DOCUMENT document, unsigned int obj_num) { if (!doc || obj_num == 0) { return nullptr; } - return LoadPageByValidatedIndex(document, doc, doc->GetPageIndex(obj_num)); + return LoadPageByValidatedIndex(document, doc, doc->GetPageIndex(obj_num), + /*normalize=*/false); +} + +FPDF_EXPORT FPDF_PAGE FPDF_CALLCONV +EPDFDoc_LoadPageByObjectNumberNormalized(FPDF_DOCUMENT document, + unsigned int obj_num) { + auto* doc = CPDFDocumentFromFPDFDocument(document); + if (!doc || obj_num == 0) { + return nullptr; + } + return LoadPageByValidatedIndex(document, doc, doc->GetPageIndex(obj_num), + /*normalize=*/true); } FPDF_EXPORT unsigned int FPDF_CALLCONV @@ -792,6 +815,68 @@ EPDFDoc_GetPageObjectNumberByIndex(FPDF_DOCUMENT document, int page_index) { return dict ? dict->GetObjNum() : 0; } +FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV +EPDFDoc_DeletePageByObjectNumber(FPDF_DOCUMENT document, unsigned int obj_num) { + auto* doc = CPDFDocumentFromFPDFDocument(document); + if (!doc || obj_num == 0) { + return false; + } + +#ifdef PDF_ENABLE_XFA + // XFA pages do not have CPDF_Page dictionaries. Match the other + // EPDFDoc_*ByObjectNumber APIs and reject object-number page mutations for + // XFA-backed documents. + if (doc->GetExtension()) { + return false; + } +#endif // PDF_ENABLE_XFA + + const int page_index = doc->GetPageIndex(obj_num); + if (page_index < 0) { + return false; + } + + const uint32_t deleted_obj_num = doc->DeletePage(page_index); + if (deleted_obj_num == 0) { + return false; + } + + doc->SetPageToNullObject(deleted_obj_num); + return true; +} + +FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV +EPDFDoc_SetPageRotationByObjectNumber(FPDF_DOCUMENT document, + unsigned int obj_num, + int rotate) { + auto* doc = CPDFDocumentFromFPDFDocument(document); + if (!doc || obj_num == 0 || rotate < 0 || rotate > 3) { + return false; + } + +#ifdef PDF_ENABLE_XFA + // XFA pages do not have CPDF_Page dictionaries. Match the other + // EPDFDoc_*ByObjectNumber APIs and reject object-number page mutations for + // XFA-backed documents. + if (doc->GetExtension()) { + return false; + } +#endif // PDF_ENABLE_XFA + + if (doc->GetPageIndex(obj_num) < 0) { + return false; + } + + RetainPtr page_dict = + ToDictionary(doc->GetMutableIndirectObject(obj_num)); + if (!page_dict) { + return false; + } + + page_dict->SetNewFor(pdfium::page_object::kRotate, rotate * 90); + return true; +} + FPDF_EXPORT unsigned int FPDF_CALLCONV EPDFPage_GetObjectNumber(FPDF_PAGE page) { // Note: CPDFPageFromFPDFPage() returns null for XFA pages, so this function diff --git a/fpdfsdk/fpdf_view_c_api_test.c b/fpdfsdk/fpdf_view_c_api_test.c index fc3678e32e..cc3ae907c3 100644 --- a/fpdfsdk/fpdf_view_c_api_test.c +++ b/fpdfsdk/fpdf_view_c_api_test.c @@ -543,7 +543,9 @@ int CheckPDFiumCApi() { CHK(EPDF_LoadBaseDocument); CHK(EPDF_LoadMemBaseDocument); CHK(EPDF_LoadMemBaseDocument64); + CHK(EPDFDoc_DeletePageByObjectNumber); CHK(EPDFDoc_GetPageObjectNumberByIndex); + CHK(EPDFDoc_SetPageRotationByObjectNumber); CHK(EPDF_FreeBuffer); CHK(EPDF_SaveDocumentToOwnedBuffer); CHK(EPDF_SaveDocumentToOwnedBufferWithVersion); diff --git a/fpdfsdk/fpdf_view_embeddertest.cpp b/fpdfsdk/fpdf_view_embeddertest.cpp index a995d382d0..bdca4837cb 100644 --- a/fpdfsdk/fpdf_view_embeddertest.cpp +++ b/fpdfsdk/fpdf_view_embeddertest.cpp @@ -25,6 +25,7 @@ #include "public/fpdf_annot.h" #include "public/fpdf_attachment.h" #include "public/fpdf_doc.h" +#include "public/fpdf_edit.h" #include "public/fpdf_javascript.h" #include "public/fpdf_save.h" #include "public/fpdf_text.h" @@ -2680,6 +2681,96 @@ TEST_F(FPDFViewEmbedderTest, EPDFDocGetPageObjectNumberByIndex) { EXPECT_EQ(1u, doc->GetParsedPageCountForTesting()); } +TEST_F(FPDFViewEmbedderTest, EPDFDocSetPageRotationByObjectNumber) { + constexpr char kRotatedPng[] = "rectangles_rotated"; + + ASSERT_TRUE(OpenDocument("rectangles.pdf")); + + const unsigned int objnum = EPDFDoc_GetPageObjectNumberByIndex(document(), 0); + ASSERT_NE(0u, objnum); + + EXPECT_FALSE(EPDFDoc_SetPageRotationByObjectNumber(nullptr, objnum, 1)); + EXPECT_FALSE(EPDFDoc_SetPageRotationByObjectNumber(document(), 0, 1)); + EXPECT_FALSE(EPDFDoc_SetPageRotationByObjectNumber(document(), objnum, -1)); + EXPECT_FALSE(EPDFDoc_SetPageRotationByObjectNumber(document(), objnum, 4)); + + CPDF_Document* doc = CPDFDocumentFromFPDFDocument(document()); + ASSERT_TRUE(doc); + EXPECT_EQ(0u, doc->GetParsedPageCountForTesting()); + + EXPECT_TRUE(EPDFDoc_SetPageRotationByObjectNumber(document(), objnum, 1)); + EXPECT_EQ(0u, doc->GetParsedPageCountForTesting()); + + { + ScopedPage page = LoadScopedPage(0); + ASSERT_TRUE(page); + EXPECT_EQ(1, FPDFPage_GetRotation(page.get())); + EXPECT_EQ(300, static_cast(FPDF_GetPageWidth(page.get()))); + EXPECT_EQ(200, static_cast(FPDF_GetPageHeight(page.get()))); + ScopedFPDFBitmap bitmap = RenderLoadedPage(page.get()); + CompareBitmapWithExpectationSuffix(bitmap.get(), kRotatedPng); + } + + ASSERT_TRUE(FPDF_SaveAsCopy(document(), this, 0)); + ScopedSavedDoc saved_document = OpenScopedSavedDocument(); + ASSERT_TRUE(saved_document); + ScopedSavedPage saved_page = LoadScopedSavedPage(0); + ASSERT_TRUE(saved_page); + EXPECT_EQ(1, FPDFPage_GetRotation(saved_page.get())); +} + +TEST_F(FPDFViewEmbedderTest, EPDFDocDeletePageByObjectNumber) { + ASSERT_TRUE(OpenDocument("rectangles_multi_pages.pdf")); + ASSERT_EQ(5, FPDF_GetPageCount(document())); + + const unsigned int original_page_1 = + EPDFDoc_GetPageObjectNumberByIndex(document(), 1); + ASSERT_NE(0u, original_page_1); + + int page_to_move = 1; + ASSERT_TRUE(FPDF_MovePages(document(), &page_to_move, 1, 4)); + ASSERT_EQ(5, FPDF_GetPageCount(document())); + EXPECT_EQ(original_page_1, EPDFDoc_GetPageObjectNumberByIndex(document(), 4)); + + EXPECT_FALSE(EPDFDoc_DeletePageByObjectNumber(nullptr, original_page_1)); + EXPECT_FALSE(EPDFDoc_DeletePageByObjectNumber(document(), 0)); + EXPECT_TRUE(EPDFDoc_DeletePageByObjectNumber(document(), original_page_1)); + EXPECT_EQ(4, FPDF_GetPageCount(document())); + + for (int i = 0; i < FPDF_GetPageCount(document()); ++i) { + EXPECT_NE(original_page_1, + EPDFDoc_GetPageObjectNumberByIndex(document(), i)); + } + + EXPECT_FALSE(EPDFDoc_DeletePageByObjectNumber(document(), original_page_1)); +} + +TEST_F(FPDFViewEmbedderTest, + EPDFDocDeletePageByObjectNumberDeletesDuplicatePageObjectOccurrence) { + // This malformed compatibility fixture references the same /Page object from + // multiple visible page positions. Delete-by-object-number removes the first + // visible occurrence resolved by PDFium. + ASSERT_TRUE(OpenDocument("bug_1229106.pdf")); + ASSERT_EQ(4, FPDF_GetPageCount(document())); + + const unsigned int duplicate_objnum = + EPDFDoc_GetPageObjectNumberByIndex(document(), 0); + ASSERT_NE(0u, duplicate_objnum); + ASSERT_EQ(duplicate_objnum, EPDFDoc_GetPageObjectNumberByIndex(document(), 1)); + + EXPECT_TRUE(EPDFDoc_DeletePageByObjectNumber(document(), duplicate_objnum)); + EXPECT_EQ(3, FPDF_GetPageCount(document())); + EXPECT_EQ(duplicate_objnum, EPDFDoc_GetPageObjectNumberByIndex(document(), 0)); + + EXPECT_TRUE(EPDFDoc_DeletePageByObjectNumber(document(), duplicate_objnum)); + EXPECT_EQ(2, FPDF_GetPageCount(document())); + + for (int i = 0; i < FPDF_GetPageCount(document()); ++i) { + EXPECT_NE(duplicate_objnum, + EPDFDoc_GetPageObjectNumberByIndex(document(), i)); + } +} + TEST_F(FPDFViewEmbedderTest, EPDFGetPageBoxByIndex) { ASSERT_TRUE(OpenDocument("rectangles.pdf")); diff --git a/public/fpdfview.h b/public/fpdfview.h index 66f8ebe992..fb1816dff3 100644 --- a/public/fpdfview.h +++ b/public/fpdfview.h @@ -1890,6 +1890,24 @@ FPDF_EXPORT FPDF_RESULT FPDF_CALLCONV FPDF_BStr_Clear(FPDF_BSTR* bstr); FPDF_EXPORT FPDF_PAGE FPDF_CALLCONV EPDFDoc_LoadPageByObjectNumber(FPDF_DOCUMENT document, unsigned int obj_num); +// Experimental EmbedPDF Extension API. +// Load a page by its PDF indirect object number with rotation normalized to +// 0 degrees. Like EPDFDoc_LoadPageByObjectNumber(), but forces the page's +// rotation override to 0 so all subsequent operations (GetPageWidth, +// annotations, text, rendering) use normalized coordinates as if the page had +// no rotation. The intrinsic rotation is surfaced separately via +// EPDF_GetPageRotationByIndex(). +// +// document - handle to the document. +// obj_num - the indirect object number of the page dictionary. +// +// Returns a handle to the loaded page, or NULL if the object number +// does not correspond to a page. The caller must close the returned +// handle with FPDF_ClosePage(). +FPDF_EXPORT FPDF_PAGE FPDF_CALLCONV +EPDFDoc_LoadPageByObjectNumberNormalized(FPDF_DOCUMENT document, + unsigned int obj_num); + // Experimental EmbedPDF Extension API. // Get the PDF indirect object number of a page's dictionary by page index. // Unlike FPDF_LoadPage(), this does not construct a page object or parse page @@ -1904,6 +1922,42 @@ EPDFDoc_LoadPageByObjectNumber(FPDF_DOCUMENT document, unsigned int obj_num); FPDF_EXPORT unsigned int FPDF_CALLCONV EPDFDoc_GetPageObjectNumberByIndex(FPDF_DOCUMENT document, int page_index); +// Experimental EmbedPDF Extension API. +// Delete the first visible page whose page dictionary has |obj_num| as its PDF +// indirect object number. Unlike FPDFPage_Delete(), this remains stable across +// page moves because it is keyed by page identity rather than page position. +// +// document - handle to the document. +// obj_num - the indirect object number of the page dictionary. +// +// Returns TRUE if a matching page was found and deleted, or FALSE if |document| +// is invalid, |obj_num| is 0, or |obj_num| does not correspond to a visible +// page. If a malformed PDF references the same /Page object more than once, +// the first visible occurrence resolved by PDFium is deleted. +FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV +EPDFDoc_DeletePageByObjectNumber(FPDF_DOCUMENT document, unsigned int obj_num); + +// Experimental EmbedPDF Extension API. +// Set the rotation of the page whose page dictionary has |obj_num| as its PDF +// indirect object number. Unlike FPDFPage_SetRotation(), this does not require +// loading or parsing the page. +// +// document - handle to the document. +// obj_num - the indirect object number of the page dictionary. +// rotate - the rotation value, one of: +// 0 - No rotation. +// 1 - Rotated 90 degrees clockwise. +// 2 - Rotated 180 degrees clockwise. +// 3 - Rotated 270 degrees clockwise. +// +// Returns TRUE if a matching page was found and updated, or FALSE if |document| +// is invalid, |obj_num| is 0, |rotate| is outside 0..3, or |obj_num| does not +// correspond to a visible page. +FPDF_EXPORT FPDF_BOOL FPDF_CALLCONV +EPDFDoc_SetPageRotationByObjectNumber(FPDF_DOCUMENT document, + unsigned int obj_num, + int rotate); + // Experimental EmbedPDF Extension API. // Get the PDF indirect object number of a page's dictionary. //