caddypki,caddytls: fix data races in CA accessors and TLS storage cleanup#7410
Open
jeffkayser2 wants to merge 1 commit intocaddyserver:masterfrom
Open
caddypki,caddytls: fix data races in CA accessors and TLS storage cleanup#7410jeffkayser2 wants to merge 1 commit intocaddyserver:masterfrom
jeffkayser2 wants to merge 1 commit intocaddyserver:masterfrom
Conversation
…anup
Fix two data races detected when running with -race flag:
1. caddypki/ca.go: The CA accessor methods (RootCertificate,
IntermediateCertificate, IntermediateKey, RootKey) used value
receivers which copy the struct before the mutex lock is acquired.
This allows concurrent goroutines to read stale copies of the CA
fields while renewCertsForCA() is writing to them.
Changed to pointer receivers so the lock is acquired on the original
struct before any field access occurs.
2. caddytls/tls.go: The keepStorageClean() method writes to
storageCleanTicker and storageCleanStop without synchronization,
racing with Stop() and CaddyModule() which may read these fields
concurrently.
Added storageCleanMu mutex to protect these fields in both
keepStorageClean() and Stop().
Race detector output before fix:
WARNING: DATA RACE
Write at 0x00c000a5ca90 by main goroutine:
github.com/caddyserver/caddy/v2/modules/caddypki.(*PKI).renewCertsForCA()
modules/caddypki/maintain.go:89
Previous read at 0x00c000a5ca90 by goroutine 176:
github.com/caddyserver/caddy/v2/modules/caddypki.(*CA).NewAuthority()
modules/caddypki/ca.go:199
WARNING: DATA RACE
Write at 0x00c000a5cc28 by main goroutine:
github.com/caddyserver/caddy/v2/modules/caddytls.(*TLS).keepStorageClean()
modules/caddytls/tls.go:789
Previous read at 0x00c000a5cc28 by goroutine 170:
github.com/caddyserver/caddy/v2/modules/caddytls.(*TLS).CaddyModule()
Related: caddyserver#4517, caddyserver#4669
|
|
Member
|
Thanks! Although, the base commit seems to be old. Would you mind fixing the merge conflict? |
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
3 participants
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.
…anup
Fix two data races detected when running with -race flag:
caddypki/ca.go: The CA accessor methods (RootCertificate, IntermediateCertificate, IntermediateKey, RootKey) used value receivers which copy the struct before the mutex lock is acquired. This allows concurrent goroutines to read stale copies of the CA fields while renewCertsForCA() is writing to them.
Changed to pointer receivers so the lock is acquired on the original struct before any field access occurs.
caddytls/tls.go: The keepStorageClean() method writes to storageCleanTicker and storageCleanStop without synchronization, racing with Stop() and CaddyModule() which may read these fields concurrently.
Added storageCleanMu mutex to protect these fields in both keepStorageClean() and Stop().
Race detector output before fix:
WARNING: DATA RACE
Write at 0x00c000a5ca90 by main goroutine:
github.com/caddyserver/caddy/v2/modules/caddypki.(*PKI).renewCertsForCA()
modules/caddypki/maintain.go:89
Previous read at 0x00c000a5ca90 by goroutine 176:
github.com/caddyserver/caddy/v2/modules/caddypki.(*CA).NewAuthority()
modules/caddypki/ca.go:199
WARNING: DATA RACE
Write at 0x00c000a5cc28 by main goroutine:
github.com/caddyserver/caddy/v2/modules/caddytls.(*TLS).keepStorageClean()
modules/caddytls/tls.go:789
Previous read at 0x00c000a5cc28 by goroutine 170:
github.com/caddyserver/caddy/v2/modules/caddytls.(*TLS).CaddyModule()
Related: #4517, #4669
Assistance Disclosure
I used Claude Code v2.0.55 Opus 4.5 - Claude Max, to fix the data race, and build the files to send to github.comThis PR is missing an assistance disclosure.