fix: clone custom font bytes per gofpdf provider to avoid race#573
Open
johnfercher-ai[bot] wants to merge 2 commits into
Open
fix: clone custom font bytes per gofpdf provider to avoid race#573johnfercher-ai[bot] wants to merge 2 commits into
johnfercher-ai[bot] wants to merge 2 commits into
Conversation
Under concurrent generation mode each worker builds its own gofpdf provider, but they were sharing the same underlying byte slice from the configured custom fonts. gofpdf mutates this slice inside putfonts / GenerateCutFont (specifically generateChecksum), which races when two workers are finalising documents at the same time. Clone the font bytes when calling AddUTF8FontFromBytes so every provider owns an independent backing array, eliminating the write/write and write/read data races reported by the Go race detector. Closes #550.
The previous commit introduced internal/providers/gofpdf/builder_internal_test.go without a package declaration or imports, which broke the gofpdf test build. Restore the file so it compiles as an external (gofpdf_test) package test and keeps guarding against the regression where Build would mutate the caller's custom-font byte slice.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #573 +/- ##
=======================================
Coverage 96.28% 96.28%
=======================================
Files 65 65
Lines 2443 2443
=======================================
Hits 2352 2352
Misses 60 60
Partials 31 31
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #550.
Under concurrent generation mode, maroto creates one gofpdf provider per worker. When a custom UTF-8 font is configured, every provider was receiving the same
[]byteslice fromcfg.CustomFonts. During PDF finalization gofpdf mutates that backing array insideutf8FontFile.generateChecksum/GenerateCutFont, which race-detects across workers. Cloning the font bytes per-provider in the gofpdf builder gives each provider its own backing array and eliminates the race.The fix is a one-liner in
internal/providers/gofpdf/builder.go(bytes.Clone(font.GetBytes())). Two regression tests guard it:internal/providers/gofpdf/builder_internal_test.go— unit-level check thatBuildnever mutates the caller's font bytes.maroto_test.go— end-to-end test that reproduces the issue's scenario (concurrent mode + custom font, many rows) and assertsGenerate()succeeds cleanly.Test plan
go test -race -count=1 -run TestBuilder_Build ./internal/providers/gofpdf/...— passes.go test -race -count=1 -run TestMaroto_Generate .— passes (includes the "when concurrent mode is active with a custom font, should not race" subtest).go test -count=1 ./...— entire suite is green.