Skip to content

Commit

Permalink
improve the handling of negative durations with 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, as users
can just return zero or a reasonably negative duration instead if oddly
intentional. The primary cause for a negative if a mistakenly ordered
Duration.between(t1, t2) call, so handling -292 years is a worthy test
case but bad user code entries are unintentionally, immediatly
discarded.
  • Loading branch information
ben-manes committed Jan 22, 2025
1 parent 439c561 commit 88226ef
Show file tree
Hide file tree
Showing 29 changed files with 520 additions and 51 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
12 changes: 7 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,13 @@ 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
dependency-caching: true
- 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 @@ -70,6 +71,7 @@ jobs:
security-and-quality,
security-experimental
languages: java-kotlin
dependency-caching: true
packs: >
+githubsecuritylab/codeql-java-queries,
githubsecuritylab/codeql-java-extensions,
Expand All @@ -78,6 +80,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.max(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
Loading

0 comments on commit 88226ef

Please sign in to comment.