luxolus
/
open-vault
2
0
Fork 0
Code Issues 1 Pull Requests Releases Activity

Fix race in PKI's runUnifiedTransfer (#20701) 

* Fix race in PKI's runUnifiedTransfer

During this race, we'll sometimes start (or fail to start) an additional
unified transfer if the updated last run timestamp was written at the
same time as another thread was reading it.

Instead, delay this check until we're holding the CAS guard; this will
occasionally result in more messages saying that an existing process is
already running, but otherwise shouldn't impact the functionality at
all.

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

* Add changelog entry

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>

---------

Signed-off-by: Alexander Scheel <alex.scheel@hashicorp.com>
This commit is contained in:
Alexander Scheel 2023-05-22 14:31:19 -04:00 committed by GitHub
parent 1e752e0cba
commit 1f21afba21
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 14 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
builtin/logical/pki/periodic.go
View File

@ -53,15 +53,6 @@ func runUnifiedTransfer(sc *storageContext) {
return return
} }
if !status.lastRun.IsZero() {
// We have run before, we only run again if we have
// been requested to forceRerun, and we haven't run since our
// minimum delay
if !(status.forceRerun.Load() && time.Since(status.lastRun) < minUnifiedTransferDelay) {
return
}
}
if !config.UnifiedCRL { if !config.UnifiedCRL {
// Feature is disabled, no need to run // Feature is disabled, no need to run
return return
@ -80,6 +71,17 @@ func runUnifiedTransfer(sc *storageContext) {
} }
defer status.isRunning.Store(false) defer status.isRunning.Store(false)
// Because access to lastRun is not locked, we need to delay this check
// until after we grab the isRunning CAS lock.
if !status.lastRun.IsZero() {
// We have run before, we only run again if we have
// been requested to forceRerun, and we haven't run since our
// minimum delay.
if !(status.forceRerun.Load() && time.Since(status.lastRun) < minUnifiedTransferDelay) {
return
}
}
// Reset our flag before we begin, we do this before we start as // Reset our flag before we begin, we do this before we start as
// we can't guarantee that we can properly parse/fix the error from an // we can't guarantee that we can properly parse/fix the error from an
// error that comes in from the revoke API after that. This will // error that comes in from the revoke API after that. This will

3
changelog/20701.txt Normal file
View File

@ -0,0 +1,3 @@
```release-notes:bug
secrets/pki: Fix race during runUnifiedTransfer when deciding to skip re-running a test within a short window.
```