Skip to content

Commit

Permalink
improve handling of negative durations from variable expiration
Browse files Browse the repository at this point in the history
A zero or negative duration returned from the user's Expiry is treated
as an immediate expiration. A negative value was being hashed into a
future slot in the second wheel, so it delayed the eviction until that
bucket is flushed. This now forces the entry to the current time's
bucket so that it gets handled immediately after being added.

I suspect that if a Long.MIN_VALUE was used that there might have been
an overflow. I didn't investigate that scenario or its impact, and
instead focused on more robust handling and tests.
  • Loading branch information
ben-manes committed Jan 21, 2025
1 parent 439c561 commit b84db69
Show file tree
Hide file tree
Showing 29 changed files with 250 additions and 50 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/actionlint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ jobs:
runs-on: ubuntu-latest
steps:
- name: Harden Runner
uses: step-security/harden-runner@c95a14d0e5bab51a9f56296a4eb0e416910cd350 # v2.10.3
uses: step-security/harden-runner@cb605e52c26070c328afc4562f0b4ada7618a84e # v2.10.4
with:
disable-sudo: true
egress-policy: block
Expand All @@ -16,7 +16,7 @@ jobs:
github.com:443
- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
- name: actionlint
uses: reviewdog/action-actionlint@f3dcc52bc6039e5d736486952379dce3e869e8a2 # v1.63.0
uses: reviewdog/action-actionlint@abd537417cf4991e1ba8e21a67b1119f4f53b8e0 # v1.64.1
env:
SHELLCHECK_OPTS: -e SC2001 -e SC2035 -e SC2046 -e SC2061 -e SC2086 -e SC2156
with:
Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ jobs:
JAVA_VERSION: 23
steps:
- name: Harden Runner
uses: step-security/harden-runner@c95a14d0e5bab51a9f56296a4eb0e416910cd350 # v2.10.3
uses: step-security/harden-runner@cb605e52c26070c328afc4562f0b4ada7618a84e # v2.10.4
with:
disable-sudo: true
egress-policy: block
Expand All @@ -45,7 +45,7 @@ jobs:
JAVA_VERSION: 23
steps:
- name: Harden Runner
uses: step-security/harden-runner@c95a14d0e5bab51a9f56296a4eb0e416910cd350 # v2.10.3
uses: step-security/harden-runner@cb605e52c26070c328afc4562f0b4ada7618a84e # v2.10.4
with:
disable-sudo: true
egress-policy: block
Expand All @@ -64,7 +64,7 @@ jobs:
JAVA_VERSION: 23
steps:
- name: Harden Runner
uses: step-security/harden-runner@c95a14d0e5bab51a9f56296a4eb0e416910cd350 # v2.10.3
uses: step-security/harden-runner@cb605e52c26070c328afc4562f0b4ada7618a84e # v2.10.4
with:
disable-sudo: true
egress-policy: block
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/benchmarks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ jobs:
JAVA_VERSION: ${{ matrix.java }}
steps:
- name: Harden Runner
uses: step-security/harden-runner@c95a14d0e5bab51a9f56296a4eb0e416910cd350 # v2.10.3
uses: step-security/harden-runner@cb605e52c26070c328afc4562f0b4ada7618a84e # v2.10.4
with:
disable-sudo: true
egress-policy: block
Expand Down
10 changes: 5 additions & 5 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ jobs:
JAVA_VERSION: ${{ matrix.java }}
steps:
- name: Harden Runner
uses: step-security/harden-runner@c95a14d0e5bab51a9f56296a4eb0e416910cd350 # v2.10.3
uses: step-security/harden-runner@cb605e52c26070c328afc4562f0b4ada7618a84e # v2.10.4
with:
disable-sudo: true
egress-policy: block
Expand Down Expand Up @@ -166,7 +166,7 @@ jobs:
JAVA_VERSION: ${{ matrix.java }}
steps:
- name: Harden Runner
uses: step-security/harden-runner@c95a14d0e5bab51a9f56296a4eb0e416910cd350 # v2.10.3
uses: step-security/harden-runner@cb605e52c26070c328afc4562f0b4ada7618a84e # v2.10.4
with:
disable-sudo: true
egress-policy: block
Expand Down Expand Up @@ -209,7 +209,7 @@ jobs:
if: (github.event_name == 'push') && (github.event.repository.fork == false)
steps:
- name: Harden Runner
uses: step-security/harden-runner@c95a14d0e5bab51a9f56296a4eb0e416910cd350 # v2.10.3
uses: step-security/harden-runner@cb605e52c26070c328afc4562f0b4ada7618a84e # v2.10.4
with:
disable-sudo: true
egress-policy: block
Expand Down Expand Up @@ -287,7 +287,7 @@ jobs:
checks: write
steps:
- name: Harden Runner
uses: step-security/harden-runner@c95a14d0e5bab51a9f56296a4eb0e416910cd350 # v2.10.3
uses: step-security/harden-runner@cb605e52c26070c328afc4562f0b4ada7618a84e # v2.10.4
with:
disable-sudo: true
egress-policy: block
Expand Down Expand Up @@ -341,7 +341,7 @@ jobs:
id-token: write
steps:
- name: Harden Runner
uses: step-security/harden-runner@c95a14d0e5bab51a9f56296a4eb0e416910cd350 # v2.10.3
uses: step-security/harden-runner@cb605e52c26070c328afc4562f0b4ada7618a84e # v2.10.4
with:
disable-sudo: true
egress-policy: block
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/codacy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ jobs:
if: github.event.repository.fork == false
steps:
- name: Harden Runner
uses: step-security/harden-runner@c95a14d0e5bab51a9f56296a4eb0e416910cd350 # v2.10.3
uses: step-security/harden-runner@cb605e52c26070c328afc4562f0b4ada7618a84e # v2.10.4
with:
disable-sudo: true
egress-policy: block
Expand Down Expand Up @@ -47,7 +47,7 @@ jobs:
if: steps.check_files.outputs.files_exists == 'true'
run: jq -c '.runs |= unique_by({tool, invocations, results})' < results.sarif > codacy.sarif
- name: Upload result to GitHub Code Scanning
uses: github/codeql-action/upload-sarif@b6a472f63d85b9c78a3ac5e89422239fc15e9b3c # v3.28.1
uses: github/codeql-action/upload-sarif@d68b2d4edb4189fd2a5366ac14e72027bd4b37dd # v3.28.2
if: steps.check_files.outputs.files_exists == 'true'
continue-on-error: true
with:
Expand Down
10 changes: 5 additions & 5 deletions .github/workflows/codeql.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ jobs:
language: [ actions, java ]
steps:
- name: Harden Runner
uses: step-security/harden-runner@c95a14d0e5bab51a9f56296a4eb0e416910cd350 # v2.10.3
uses: step-security/harden-runner@cb605e52c26070c328afc4562f0b4ada7618a84e # v2.10.4
with:
disable-sudo: true
egress-policy: block
Expand Down Expand Up @@ -56,12 +56,12 @@ jobs:
java: ${{ env.JAVA_VERSION }}
cache-encryption-key: ${{ secrets.GRADLE_ENCRYPTION_KEY }}
- name: Initialize CodeQL (Actions)
uses: github/codeql-action/init@b6a472f63d85b9c78a3ac5e89422239fc15e9b3c # v3.28.1
uses: github/codeql-action/init@d68b2d4edb4189fd2a5366ac14e72027bd4b37dd # v3.28.2
if: ${{ matrix.language == 'actions' }}
with:
languages: actions
- name: Initialize CodeQL (Java)
uses: github/codeql-action/init@b6a472f63d85b9c78a3ac5e89422239fc15e9b3c # v3.28.1
uses: github/codeql-action/init@d68b2d4edb4189fd2a5366ac14e72027bd4b37dd # v3.28.2
if: ${{ matrix.language == 'java' }}
with:
queries: >
Expand All @@ -78,6 +78,6 @@ jobs:
config: |
threat-models: local
- name: Autobuild
uses: github/codeql-action/autobuild@b6a472f63d85b9c78a3ac5e89422239fc15e9b3c # v3.28.1
uses: github/codeql-action/autobuild@d68b2d4edb4189fd2a5366ac14e72027bd4b37dd # v3.28.2
- name: Perform CodeQL Analysis
uses: github/codeql-action/analyze@b6a472f63d85b9c78a3ac5e89422239fc15e9b3c # v3.28.1
uses: github/codeql-action/analyze@d68b2d4edb4189fd2a5366ac14e72027bd4b37dd # v3.28.2
4 changes: 2 additions & 2 deletions .github/workflows/dependency-check.yml
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ jobs:
&& (github.event_name != 'pull_request' || github.event.pull_request.head.repo.fork == false)
steps:
- name: Harden Runner
uses: step-security/harden-runner@c95a14d0e5bab51a9f56296a4eb0e416910cd350 # v2.10.3
uses: step-security/harden-runner@cb605e52c26070c328afc4562f0b4ada7618a84e # v2.10.4
with:
disable-sudo: true
egress-policy: block
Expand Down Expand Up @@ -61,7 +61,7 @@ jobs:
with:
files: build/reports/dependency-check-report.sarif
- name: Upload result to GitHub Code Scanning
uses: github/codeql-action/upload-sarif@b6a472f63d85b9c78a3ac5e89422239fc15e9b3c # v3.28.1
uses: github/codeql-action/upload-sarif@d68b2d4edb4189fd2a5366ac14e72027bd4b37dd # v3.28.2
if: steps.check_files.outputs.files_exists == 'true'
with:
sarif_file: build/reports/dependency-check-report.sarif
2 changes: 1 addition & 1 deletion .github/workflows/dependency-review.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ jobs:
pull-requests: write
steps:
- name: Harden Runner
uses: step-security/harden-runner@c95a14d0e5bab51a9f56296a4eb0e416910cd350 # v2.10.3
uses: step-security/harden-runner@cb605e52c26070c328afc4562f0b4ada7618a84e # v2.10.4
with:
disable-sudo: true
egress-policy: block
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/dependency-submission-pr-retreive.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ jobs:
contents: write
steps:
- name: Harden Runner
uses: step-security/harden-runner@c95a14d0e5bab51a9f56296a4eb0e416910cd350 # v2.10.3
uses: step-security/harden-runner@cb605e52c26070c328afc4562f0b4ada7618a84e # v2.10.4
with:
disable-sudo: true
egress-policy: block
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/dependency-submission-pr-submit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ jobs:
contents: read
steps:
- name: Harden Runner
uses: step-security/harden-runner@c95a14d0e5bab51a9f56296a4eb0e416910cd350 # v2.10.3
uses: step-security/harden-runner@cb605e52c26070c328afc4562f0b4ada7618a84e # v2.10.4
with:
disable-sudo: true
egress-policy: block
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/dependency-submission.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ jobs:
contents: write
steps:
- name: Harden Runner
uses: step-security/harden-runner@c95a14d0e5bab51a9f56296a4eb0e416910cd350 # v2.10.3
uses: step-security/harden-runner@cb605e52c26070c328afc4562f0b4ada7618a84e # v2.10.4
with:
disable-sudo: true
egress-policy: block
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/devskim.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ jobs:
security-events: write
steps:
- name: Harden Runner
uses: step-security/harden-runner@c95a14d0e5bab51a9f56296a4eb0e416910cd350 # v2.10.3
uses: step-security/harden-runner@cb605e52c26070c328afc4562f0b4ada7618a84e # v2.10.4
with:
disable-sudo: true
egress-policy: block
Expand All @@ -31,6 +31,6 @@ jobs:
- name: Run DevSkim scanner
uses: microsoft/DevSkim-Action@914fa647b406c387000300b2f09bb28691be2b6d # v1.0.14
- name: Upload DevSkim scan results to GitHub Security tab
uses: github/codeql-action/upload-sarif@b6a472f63d85b9c78a3ac5e89422239fc15e9b3c # v3.28.1
uses: github/codeql-action/upload-sarif@d68b2d4edb4189fd2a5366ac14e72027bd4b37dd # v3.28.2
with:
sarif_file: devskim-results.sarif
2 changes: 1 addition & 1 deletion .github/workflows/examples.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ jobs:
runs-on: ubuntu-latest
steps:
- name: Harden Runner
uses: step-security/harden-runner@c95a14d0e5bab51a9f56296a4eb0e416910cd350 # v2.10.3
uses: step-security/harden-runner@cb605e52c26070c328afc4562f0b4ada7618a84e # v2.10.4
with:
disable-sudo: true
egress-policy: block
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/gitleaks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ jobs:
runs-on: ubuntu-latest
steps:
- name: Harden Runner
uses: step-security/harden-runner@c95a14d0e5bab51a9f56296a4eb0e416910cd350 # v2.10.3
uses: step-security/harden-runner@cb605e52c26070c328afc4562f0b4ada7618a84e # v2.10.4
with:
disable-sudo: true
egress-policy: block
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/gradle-wrapper-validation.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ jobs:
runs-on: ubuntu-latest
steps:
- name: Harden Runner
uses: step-security/harden-runner@c95a14d0e5bab51a9f56296a4eb0e416910cd350 # v2.10.3
uses: step-security/harden-runner@cb605e52c26070c328afc4562f0b4ada7618a84e # v2.10.4
with:
disable-sudo: true
egress-policy: block
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/qodana.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ jobs:
&& (github.event_name != 'pull_request' || github.event.pull_request.head.repo.fork == false)
steps:
- name: Harden Runner
uses: step-security/harden-runner@c95a14d0e5bab51a9f56296a4eb0e416910cd350 # v2.10.3
uses: step-security/harden-runner@cb605e52c26070c328afc4562f0b4ada7618a84e # v2.10.4
with:
disable-sudo: true
egress-policy: block
Expand Down Expand Up @@ -70,6 +70,6 @@ jobs:
upload-result: true
github-token: ${{ secrets.GITHUB_TOKEN }}
- name: Upload SARIF file for GitHub Advanced Security Dashboard
uses: github/codeql-action/upload-sarif@b6a472f63d85b9c78a3ac5e89422239fc15e9b3c # v3.28.1
uses: github/codeql-action/upload-sarif@d68b2d4edb4189fd2a5366ac14e72027bd4b37dd # v3.28.2
with:
sarif_file: ${{ runner.temp }}/qodana/results/qodana.sarif.json
2 changes: 1 addition & 1 deletion .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ jobs:
id-token: write
steps:
- name: Harden Runner
uses: step-security/harden-runner@c95a14d0e5bab51a9f56296a4eb0e416910cd350 # v2.10.3
uses: step-security/harden-runner@cb605e52c26070c328afc4562f0b4ada7618a84e # v2.10.4
with:
disable-sudo: true
egress-policy: audit
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/scorecards-analysis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ jobs:
if: github.event.repository.fork == false
steps:
- name: Harden Runner
uses: step-security/harden-runner@c95a14d0e5bab51a9f56296a4eb0e416910cd350 # v2.10.3
uses: step-security/harden-runner@cb605e52c26070c328afc4562f0b4ada7618a84e # v2.10.4
with:
disable-sudo: true
egress-policy: block
Expand Down Expand Up @@ -58,6 +58,6 @@ jobs:
path: results.sarif
retention-days: 5
- name: Upload to code-scanning
uses: github/codeql-action/upload-sarif@b6a472f63d85b9c78a3ac5e89422239fc15e9b3c # v3.28.1
uses: github/codeql-action/upload-sarif@d68b2d4edb4189fd2a5366ac14e72027bd4b37dd # v3.28.2
with:
sarif_file: results.sarif
2 changes: 1 addition & 1 deletion .github/workflows/semgrep.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ jobs:
with:
files: results.sarif
- name: Upload SARIF file for GitHub Advanced Security Dashboard
uses: github/codeql-action/upload-sarif@b6a472f63d85b9c78a3ac5e89422239fc15e9b3c # v3.28.1
uses: github/codeql-action/upload-sarif@d68b2d4edb4189fd2a5366ac14e72027bd4b37dd # v3.28.2
if: steps.check_files.outputs.files_exists == 'true'
continue-on-error: true
with:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/snyk.yml
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ jobs:
with:
files: snyk.sarif
- name: Upload result to GitHub Code Scanning
uses: github/codeql-action/upload-sarif@b6a472f63d85b9c78a3ac5e89422239fc15e9b3c # v3.28.1
uses: github/codeql-action/upload-sarif@d68b2d4edb4189fd2a5366ac14e72027bd4b37dd # v3.28.2
if: steps.check_files.outputs.files_exists == 'true'
with:
sarif_file: snyk.sarif
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/spelling.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ jobs:
runs-on: ubuntu-latest
steps:
- name: Harden Runner
uses: step-security/harden-runner@c95a14d0e5bab51a9f56296a4eb0e416910cd350 # v2.10.3
uses: step-security/harden-runner@cb605e52c26070c328afc4562f0b4ada7618a84e # v2.10.4
with:
disable-sudo: true
egress-policy: block
Expand All @@ -25,7 +25,7 @@ jobs:
runs-on: ubuntu-latest
steps:
- name: Harden Runner
uses: step-security/harden-runner@c95a14d0e5bab51a9f56296a4eb0e416910cd350 # v2.10.3
uses: step-security/harden-runner@cb605e52c26070c328afc4562f0b4ada7618a84e # v2.10.4
with:
disable-sudo: true
egress-policy: block
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/trivy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ jobs:
security-events: write
steps:
- name: Harden Runner
uses: step-security/harden-runner@c95a14d0e5bab51a9f56296a4eb0e416910cd350 # v2.10.3
uses: step-security/harden-runner@cb605e52c26070c328afc4562f0b4ada7618a84e # v2.10.4
with:
disable-sudo: true
egress-policy: block
Expand All @@ -37,7 +37,7 @@ jobs:
with:
files: results.sarif
- name: Upload result to GitHub Code Scanning
uses: github/codeql-action/upload-sarif@b6a472f63d85b9c78a3ac5e89422239fc15e9b3c # v3.28.1
uses: github/codeql-action/upload-sarif@d68b2d4edb4189fd2a5366ac14e72027bd4b37dd # v3.28.2
if: steps.check_files.outputs.files_exists == 'true'
with:
sarif_file: results.sarif
Original file line number Diff line number Diff line change
Expand Up @@ -1430,7 +1430,7 @@ boolean skipReadBuffer() {
long expireAfterCreate(@Nullable K key, @Nullable V value,
Expiry<? super K, ? super V> expiry, long now) {
if (expiresVariable() && (key != null) && (value != null)) {
long duration = expiry.expireAfterCreate(key, value, now);
long duration = Math.max(0L, expiry.expireAfterCreate(key, value, now));
return isAsync ? (now + duration) : (now + Math.min(duration, MAXIMUM_EXPIRY));
}
return 0L;
Expand All @@ -1450,7 +1450,7 @@ long expireAfterUpdate(Node<K, V> node, @Nullable K key,
@Nullable V value, Expiry<? super K, ? super V> expiry, long now) {
if (expiresVariable() && (key != null) && (value != null)) {
long currentDuration = Math.max(1, node.getVariableTime() - now);
long duration = expiry.expireAfterUpdate(key, value, now, currentDuration);
long duration = Math.max(0L, expiry.expireAfterUpdate(key, value, now, currentDuration));
return isAsync ? (now + duration) : (now + Math.min(duration, MAXIMUM_EXPIRY));
}
return 0L;
Expand All @@ -1469,8 +1469,8 @@ long expireAfterUpdate(Node<K, V> node, @Nullable K key,
long expireAfterRead(Node<K, V> node, @Nullable K key,
@Nullable V value, Expiry<K, V> expiry, long now) {
if (expiresVariable() && (key != null) && (value != null)) {
long currentDuration = Math.max(1, node.getVariableTime() - now);
long duration = expiry.expireAfterRead(key, value, now, currentDuration);
long currentDuration = Math.max(0L, node.getVariableTime() - now);
long duration = Math.min(0L, expiry.expireAfterRead(key, value, now, currentDuration));
return isAsync ? (now + duration) : (now + Math.min(duration, MAXIMUM_EXPIRY));
}
return 0L;
Expand Down Expand Up @@ -1498,7 +1498,7 @@ void tryExpireAfterRead(Node<K, V> node, @Nullable K key,
return;
}

long duration = expiry.expireAfterRead(key, value, now, currentDuration);
long duration = Math.max(0L, expiry.expireAfterRead(key, value, now, currentDuration));
if (duration != currentDuration) {
long expirationTime = isAsync ? (now + duration) : (now + Math.min(duration, MAXIMUM_EXPIRY));
node.casVariableTime(variableTime, expirationTime);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,12 @@ public void deschedule(Node<K, V> node) {
* @return the sentinel at the head of the bucket
*/
@SuppressWarnings("Varifier")
Node<K, V> findBucket(long time) {
long duration = time - nanos;
Node<K, V> findBucket(@Var long time) {
long duration = Math.max(0L, time - nanos);
if (duration <= 0L) {

Check notice on line 209 in caffeine/src/main/java/com/github/benmanes/caffeine/cache/TimerWheel.java

View workflow job for this annotation

GitHub Actions / Qodana Community for JVM

Non-strict inequality '>=' or '<=' can be replaced with '=='

Can be replaced with equality
time = nanos;
}

int length = wheel.length - 1;
for (int i = 0; i < length; i++) {
if (duration < SPANS[i + 1]) {
Expand Down
Loading

0 comments on commit b84db69

Please sign in to comment.