From b9a1168f2d1fcf82d6bdec2ff329462638d63bb1 Mon Sep 17 00:00:00 2001 From: rsaavedraf <40606625+rsaavedraf@users.noreply.github.com> Date: Tue, 26 Jun 2018 21:14:19 +0200 Subject: [PATCH 1/2] Debugging PriorityQueue.java The change I'm proposing eliminates a problem by properly checking the validity of maxSize itself, before even computing heapSize=maxSize+1. In the constructor, when maxSize has a value == Integer.MAX_VALUE, then heapSize = maxSize+1 ends up being negative, more exactly -2147483648 (aka. Integer.MIN_VALUE.) This is quite a bug because then the if statement checking that heapSize was larger than ArrayUtil.MAX_ARRAY_LENGTH-1 ends up false, so the IllegalArgumentException is never thrown. Yet the code fails immediately afterwards when reaching "new Object[heapSize]" because of heapSize being negative, causing a NegativeArraySize exception. We saw this problem with our software which was using Solr 6.6.2. --- .../java/org/apache/lucene/util/PriorityQueue.java | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/lucene/core/src/java/org/apache/lucene/util/PriorityQueue.java b/lucene/core/src/java/org/apache/lucene/util/PriorityQueue.java index 83ac613b676a..beb7c8c301ef 100644 --- a/lucene/core/src/java/org/apache/lucene/util/PriorityQueue.java +++ b/lucene/core/src/java/org/apache/lucene/util/PriorityQueue.java @@ -50,14 +50,15 @@ public PriorityQueue(int maxSize, boolean prepopulate) { // We allocate 1 extra to avoid if statement in top() heapSize = 2; } else { - // NOTE: we add +1 because all access to heap is - // 1-based not 0-based. heap[0] is unused. - heapSize = maxSize + 1; - if (heapSize > ArrayUtil.MAX_ARRAY_LENGTH) { + if ((maxSize < 0) || (maxSize >= ArrayUtil.MAX_ARRAY_LENGTH)) { // Throw exception to prevent confusing OOME: - throw new IllegalArgumentException("maxSize must be <= " + (ArrayUtil.MAX_ARRAY_LENGTH-1) + "; got: " + maxSize); + throw new IllegalArgumentException("maxSize must be positive and < " + (ArrayUtil.MAX_ARRAY_LENGTH) + "; got: " + maxSize); } + + // NOTE: we add +1 because all access to heap is + // 1-based not 0-based. heap[0] is unused. + heapSize = maxSize + 1; } // T is unbounded type, so this unchecked cast works always: @SuppressWarnings("unchecked") final T[] h = (T[]) new Object[heapSize]; From 0a1441e7d36c836f131f2ab992bbd184e06c6653 Mon Sep 17 00:00:00 2001 From: rsaavedraf <40606625+rsaavedraf@users.noreply.github.com> Date: Thu, 28 Jun 2018 10:08:58 +0200 Subject: [PATCH 2/2] Update PriorityQueue.java --- lucene/core/src/java/org/apache/lucene/util/PriorityQueue.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lucene/core/src/java/org/apache/lucene/util/PriorityQueue.java b/lucene/core/src/java/org/apache/lucene/util/PriorityQueue.java index beb7c8c301ef..ef08c4abb1c8 100644 --- a/lucene/core/src/java/org/apache/lucene/util/PriorityQueue.java +++ b/lucene/core/src/java/org/apache/lucene/util/PriorityQueue.java @@ -53,7 +53,7 @@ public PriorityQueue(int maxSize, boolean prepopulate) { if ((maxSize < 0) || (maxSize >= ArrayUtil.MAX_ARRAY_LENGTH)) { // Throw exception to prevent confusing OOME: - throw new IllegalArgumentException("maxSize must be positive and < " + (ArrayUtil.MAX_ARRAY_LENGTH) + "; got: " + maxSize); + throw new IllegalArgumentException("maxSize must be >= 0 and < " + (ArrayUtil.MAX_ARRAY_LENGTH) + "; got: " + maxSize); } // NOTE: we add +1 because all access to heap is