Skip to content

fix: stop concurrent PDFs failing with "Temporary files directory is not writable" (6.17) - #1733

Merged
jakejackson1 merged 1 commit into
hot-patch-6.17.1from
fix/mpdf-cache-directory-race-6.17
Sep 17, 2026
Merged

jakejackson1 merged 1 commit into
hot-patch-6.17.1from
fix/mpdf-cache-directory-race-6.17

Conversation

@jakejackson1

@jakejackson1 jakejackson1 commented Sep 17, 2026 •

Copy link
Copy Markdown
Member

When two requests generate a PDF at the same moment and mPDF's cache directory doesn't exist yet, one of them can fail with Temporary files directory ".../tmp/mpdf/mpdf" is not writable. Both requests try to create the directory. wp_mkdir_p() returns false for the one that loses, and our Cache override passed that straight back to mPDF, which then throws for a directory that exists and is writable. A notification sent during that request goes out without its PDF.

GFPDF\Helper\Mpdf\Cache::createDirectory() now treats a directory that exists afterwards as created. This is the plugin's own code path: since 6.13.0 the scoper patch swaps mPDF's Cache for this subclass, so the fix doesn't need the mPDF fork.

This is the likely cause of the customer failures behind #1731 and #1732. Their log doesn't prove it, because no second generation was logged at the time of either failure.

Try it

Run from the WordPress root on a site with Gravity PDF active. It starts four wp eval-file processes at once, 20 times, each building the cache on a fresh directory:

U=$(wp eval 'echo wp_upload_dir()["basedir"];')
cat > "$U/gpdf-race.php" <<'PHP'
<?php
[ $base, $go ] = $args;
while ( ! file_exists( $go ) ) { usleep( 100 ); }
try { new GFPDF\Helper\Mpdf\Cache( $base . '/mpdf' ); echo "ok\n"; } catch ( Exception $e ) { echo $e->getMessage(), "\n"; }
PHP
for round in $(seq 20); do
  rm -rf "$U/gpdf-race" "$U/gpdf-go"; mkdir -p "$U/gpdf-race"
  for w in 1 2 3 4; do wp eval-file "$U/gpdf-race.php" "$U/gpdf-race" "$U/gpdf-go" 2>/dev/null & done
  sleep 2; touch "$U/gpdf-go"; wait
done | sort | uniq -c
rm -rf "$U/gpdf-race" "$U/gpdf-go" "$U/gpdf-race.php"

On 6.17.0 a share of the attempts print Temporary files directory ".../gpdf-race/mpdf" is not writable (24 of 80 on my dev site). With this branch every attempt prints ok.

Test plan

  • The script above prints only ok
  • PDFs still generate, and a fresh tmp/mpdf/mpdf directory is created with the parent directory's permissions
  • Test_Cache passes
More info

Where the race is. wp_mkdir_p() returns early when file_exists() is true. Otherwise it walks up to an existing parent, reads its permissions and calls @mkdir( $target, $dir_perms, true ), returning false if that fails. A request that saw the directory missing, then lost the mkdir() to another request, gets false. mPDF's Cache::createBasePath() then returns false and the constructor throws. mPDF builds two of these per Mpdf instance (tempDir/mpdf and tempDir/mpdf/ttfontdata), and both go through this override.

Measured. Four concurrent wp eval-file processes building GFPDF\Helper\Mpdf\Cache on a missing directory, on the dev site (PHP 8.5, WordPress in wp-env): 16 of 40 rounds had a failure with the current override, 0 of 40 with this change applied.

Test. test_createDirectory_accepts_a_directory_created_concurrently registers a stream wrapper over the real filesystem whose mkdir() creates the directory and then reports failure, which is what the losing request sees. With the old override it throws the customer's exact message; with this change the Cache is built. The existing permission-inheritance cases still pass. Test_Cache, Test_PDF, Test_Helper_Mpdf, Test_Slow_PDF and the font tests pass locally (168 tests, 1 skipped). PHPCS is clean.

Upstream. mPDF's own Cache::createDirectory() has the same race and is tracked in GravityPDF/mpdf#203. It doesn't affect Gravity PDF once this merges, because the override replaces that method.

…not writable"

When two requests generate a PDF while mPDF's cache directory is missing, both
try to create it. wp_mkdir_p() checks the path, then mkdir()s it, and returns
false if another request created it in between. Gravity PDF's Cache override
passed that false straight back to mPDF, which threw "Temporary files directory
.../tmp/mpdf/mpdf is not writable" for a directory that exists and is writable.

createDirectory() now counts a directory that exists afterwards as created.

Against the dev site, four concurrent wp eval-file processes building the Cache
failed in 16 of 40 rounds before this change and none after.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jakejackson1
jakejackson1 merged commit 53a4d8f into hot-patch-6.17.1 Sep 17, 2026
11 of 14 checks passed
@jakejackson1
jakejackson1 deleted the fix/mpdf-cache-directory-race-6.17 branch September 17, 2026 01:58
jakejackson1 added a commit that referenced this pull request Sep 21, 2026
Ports the 6.17.1 release (6.17..6.17.1) to development:

- keep mPDF's cache folders through tmp cleanup, and keep font metrics for a week (#1731)
- stop concurrent PDFs failing with "Temporary files directory is not writable" (#1733)
- note a PDF left off a notification on the entry, linked to its settings and authored as the
  notification, with the Gravity PDF logo as its avatar (#1732, #1736)
- log generation errors with form/entry/PDF IDs instead of the whole object (#1732)
- licensing environment type, inactive license status and dead update packages (#1734)
- tag mPDF's log records with the form, entry and PDF they belong to (#1738)
- keep PDF URL paths and safe query args when redacting logs (#1737)
- retry license reactivation soon when the store gives no verdict (#1739)

Adapted to development: tests moved to tests/phpunit/integration on the shared TestCase,
Context_Logger uses the scoped GFPDF_Vendor\Psr\Log, the generation-error log context lives in
development's refactored Model_PDF::process_and_save_pdf(), and pdf_id defaults to '' for a
Helper_PDF built without a PDF ID. Adds the 6.17.1 changelog section. Left out: the version bump and the wp-env 11
CI change (development fixed the Debian 11 build its own way in #1724).

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
jakejackson1 added a commit that referenced this pull request Sep 21, 2026
Ports the 6.17.1 release (6.17..6.17.1) to development:

- keep mPDF's cache folders through tmp cleanup, and keep font metrics for a week (#1731)
- stop concurrent PDFs failing with "Temporary files directory is not writable" (#1733)
- note a PDF left off a notification on the entry, linked to its settings and authored as the
  notification, with the Gravity PDF logo as its avatar (#1732, #1736)
- log generation errors with form/entry/PDF IDs instead of the whole object (#1732)
- licensing environment type, inactive license status and dead update packages (#1734)
- tag mPDF's log records with the form, entry and PDF they belong to (#1738)
- keep PDF URL paths and safe query args when redacting logs (#1737)
- retry license reactivation soon when the store gives no verdict (#1739)

Adapted to development: tests moved to tests/phpunit/integration on the shared TestCase,
Context_Logger uses the scoped GFPDF_Vendor\Psr\Log, the generation-error log context lives in
development's refactored Model_PDF::process_and_save_pdf(), and pdf_id defaults to '' for a
Helper_PDF built without a PDF ID. Adds the 6.17.1 changelog section. Left out: the version bump.

Also takes 6.17.1's wp-env upgrade (^11.15.0, which repoints Debian 11 sources at archive.debian.org
itself) in place of #1724's tools/wp-env/patch-bullseye-apt.mjs, which the yarn wp-env scripts no longer run.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant