Skip to content

Commit

Permalink
Fix overflow bug in SortingNumericDocValues (#70154)
Browse files Browse the repository at this point in the history
  • Loading branch information
iverase committed Mar 10, 2021
1 parent 5e487fd commit c3f11f2
Show file tree
Hide file tree
Showing 2 changed files with 85 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -60,25 +60,35 @@ protected final void resize(int newSize) {
count = newSize;
valuesCursor = 0;

if (newSize <= values.length) {
if (newSize <= getArrayLength()) {
return;
}

// Array is expected to grow so increment the circuit breaker
// to include both the additional bytes used by the grown array
// as well as the overhead of keeping both arrays in memory while
// copying.
long oldValuesSizeInBytes = values.length * Long.BYTES;
long oldValuesSizeInBytes = (long) getArrayLength() * Long.BYTES;
int newValuesLength = ArrayUtil.oversize(newSize, Long.BYTES);
circuitBreakerConsumer.accept(newValuesLength * Long.BYTES);
circuitBreakerConsumer.accept((long) newValuesLength * Long.BYTES);

// resize
values = ArrayUtil.growExact(values, newValuesLength);
growExact(newValuesLength);

// account for freeing the old values array
circuitBreakerConsumer.accept(-oldValuesSizeInBytes);
}

/** Grow the array in a method so we can override it during testing */
protected void growExact(int newValuesLength) {
values = ArrayUtil.growExact(values, newValuesLength);
}

/** Get the size of the internal array using a method so we can override it during testing */
protected int getArrayLength() {
return values.length;
}

/**
* Sort values that are stored between offsets <code>0</code> and
* {@link #count} of {@link #values}.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the Elastic License
* 2.0 and the Server Side Public License, v 1; you may not use this file except
* in compliance with, at your election, the Elastic License 2.0 or the Server
* Side Public License, v 1.
*/

package org.elasticsearch.index.fielddata;

import org.apache.lucene.util.ArrayUtil;
import org.elasticsearch.test.ESTestCase;
import org.hamcrest.Matchers;

import java.util.concurrent.atomic.AtomicLong;
import java.util.function.LongConsumer;

public class SortingNumericDocValuesTests extends ESTestCase {

public void testResize() {
final int oldSize = Integer.MAX_VALUE - 200;
final int newSize = Integer.MAX_VALUE - 100;
// This counter should account for the initialization of the array (size == 1)
// and the diff between newSize (over-sized) and oldSize.
final AtomicLong counter = new AtomicLong();
LongConsumer consumer = value -> {
long total = counter.addAndGet(value);
assertThat(total, Matchers.greaterThanOrEqualTo(0L));
};
SortingNumericDocValues docValues = new SortingNumericDocValues(consumer) {

@Override
protected void growExact(int newValuesLength) {
// don't grow the array
}

/** Get the size of the internal array using a method so we can override it during testing */
protected int getArrayLength() {
return oldSize;
}

@Override
public boolean advanceExact(int target) {
return false;
}

@Override
public int docID() {
return 0;
}

@Override
public int nextDoc() {
return 0;
}

@Override
public int advance(int target) {
return 0;
}

@Override
public long cost() {
return 0;
}
};
docValues.resize(newSize);
final long diff = ArrayUtil.oversize(newSize, Long.BYTES) - oldSize;
assertThat(counter.get(), Matchers.equalTo((diff + 1) * Long.BYTES));
}
}

0 comments on commit c3f11f2

Please sign in to comment.