Skip to content

Commit a94744f

Browse files
authored
Fix deadlock between Cache.put and invalidateAll (#99480) (#99579)
The invalidateAll method is taking out the lru lock and segment locks in a different order to the put method, when the put is replacing an existing value. This results in a deadlock between the two methods as they try to swap locks. This fixes it by making sure invalidateAll takes out locks in the same order as put. This is difficult to test because the put needs to be replacing an existing value, and invalidateAll clears the cache, resulting in subsequent puts not hitting the deadlock condition. A test that overrides some internal implementations to expose this particular deadlock will be coming later.
1 parent 5196f81 commit a94744f

File tree

2 files changed

+17
-11
lines changed

2 files changed

+17
-11
lines changed

docs/changelog/99480.yaml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
pr: 99480
2+
summary: Fix deadlock between Cache.put and Cache.invalidateAll
3+
area: Infra/Core
4+
type: bug
5+
issues:
6+
- 99326

server/src/main/java/org/elasticsearch/common/cache/Cache.java

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -521,12 +521,12 @@ public void invalidateAll() {
521521
Entry<K, V> h;
522522

523523
boolean[] haveSegmentLock = new boolean[NUMBER_OF_SEGMENTS];
524-
try {
525-
for (int i = 0; i < NUMBER_OF_SEGMENTS; i++) {
526-
segments[i].segmentLock.writeLock().lock();
527-
haveSegmentLock[i] = true;
528-
}
529-
try (ReleasableLock ignored = lruLock.acquire()) {
524+
try (ReleasableLock ignored = lruLock.acquire()) {
525+
try {
526+
for (int i = 0; i < NUMBER_OF_SEGMENTS; i++) {
527+
segments[i].segmentLock.writeLock().lock();
528+
haveSegmentLock[i] = true;
529+
}
530530
h = head;
531531
for (CacheSegment segment : segments) {
532532
segment.map = null;
@@ -539,11 +539,11 @@ public void invalidateAll() {
539539
head = tail = null;
540540
count = 0;
541541
weight = 0;
542-
}
543-
} finally {
544-
for (int i = NUMBER_OF_SEGMENTS - 1; i >= 0; i--) {
545-
if (haveSegmentLock[i]) {
546-
segments[i].segmentLock.writeLock().unlock();
542+
} finally {
543+
for (int i = NUMBER_OF_SEGMENTS - 1; i >= 0; i--) {
544+
if (haveSegmentLock[i]) {
545+
segments[i].segmentLock.writeLock().unlock();
546+
}
547547
}
548548
}
549549
}

0 commit comments

Comments
 (0)