Iterator infinite scan bug fixed #885

pull/903/head
Nikita 8 years ago
parent 4ef3d60ac7
commit 803fc814e8

@ -42,7 +42,6 @@ abstract class RedissonBaseIterator<V> implements Iterator<V> {
private boolean finished; private boolean finished;
private boolean currentElementRemoved; private boolean currentElementRemoved;
private boolean removeExecuted;
private V value; private V value;
@Override @Override
@ -53,7 +52,6 @@ abstract class RedissonBaseIterator<V> implements Iterator<V> {
free(lastValues); free(lastValues);
currentElementRemoved = false; currentElementRemoved = false;
removeExecuted = false;
client = null; client = null;
firstValues = null; firstValues = null;
lastValues = null; lastValues = null;
@ -64,9 +62,7 @@ abstract class RedissonBaseIterator<V> implements Iterator<V> {
} }
finished = false; finished = false;
} }
long prevIterPos;
do { do {
prevIterPos = nextIterPos;
ListScanResult<ScanObjectEntry> res = iterator(client, nextIterPos); ListScanResult<ScanObjectEntry> res = iterator(client, nextIterPos);
if (lastValues != null) { if (lastValues != null) {
free(lastValues); free(lastValues);
@ -82,7 +78,6 @@ abstract class RedissonBaseIterator<V> implements Iterator<V> {
client = null; client = null;
firstValues = null; firstValues = null;
nextIterPos = 0; nextIterPos = 0;
prevIterPos = -1;
} }
} else { } else {
if (firstValues.isEmpty()) { if (firstValues.isEmpty()) {
@ -93,39 +88,30 @@ abstract class RedissonBaseIterator<V> implements Iterator<V> {
client = null; client = null;
firstValues = null; firstValues = null;
nextIterPos = 0; nextIterPos = 0;
prevIterPos = -1;
continue; continue;
} }
if (res.getPos() == 0) { if (res.getPos() == 0) {
free(firstValues);
free(lastValues);
finished = true; finished = true;
return false; return false;
} }
} }
} else if (lastValues.removeAll(firstValues)) { } else if (lastValues.removeAll(firstValues)
|| (lastValues.isEmpty() && nextIterPos == 0)) {
free(firstValues); free(firstValues);
free(lastValues); free(lastValues);
currentElementRemoved = false; currentElementRemoved = false;
removeExecuted = false;
client = null; client = null;
firstValues = null; firstValues = null;
lastValues = null; lastValues = null;
nextIterPos = 0; nextIterPos = 0;
prevIterPos = -1;
if (tryAgain()) { if (tryAgain()) {
continue; continue;
} }
finished = true;
return false;
} else if (lastValues.isEmpty() && res.getPos() == 0) {
if (tryAgain()) {
client = null;
firstValues = null;
nextIterPos = 0;
prevIterPos = -1;
continue;
}
finished = true; finished = true;
return false; return false;
@ -133,10 +119,7 @@ abstract class RedissonBaseIterator<V> implements Iterator<V> {
} }
lastIter = res.getValues().iterator(); lastIter = res.getValues().iterator();
nextIterPos = res.getPos(); nextIterPos = res.getPos();
} while (!lastIter.hasNext() && nextIterPos != prevIterPos); } while (!lastIter.hasNext());
if (prevIterPos == nextIterPos && !removeExecuted) {
finished = true;
}
} }
return lastIter.hasNext(); return lastIter.hasNext();
} }
@ -188,7 +171,6 @@ abstract class RedissonBaseIterator<V> implements Iterator<V> {
lastIter.remove(); lastIter.remove();
remove(value); remove(value);
currentElementRemoved = true; currentElementRemoved = true;
removeExecuted = true;
} }
abstract void remove(V value); abstract void remove(V value);

@ -28,6 +28,14 @@ import org.redisson.client.protocol.decoder.ScanObjectEntry;
import io.netty.buffer.ByteBuf; import io.netty.buffer.ByteBuf;
/**
*
* @author Nikita Koksharov
*
* @param <K> key type
* @param <V> value type
* @param <M> loaded value type
*/
public abstract class RedissonBaseMapIterator<K, V, M> implements Iterator<M> { public abstract class RedissonBaseMapIterator<K, V, M> implements Iterator<M> {
private Map<ByteBuf, ByteBuf> firstValues; private Map<ByteBuf, ByteBuf> firstValues;
@ -38,7 +46,6 @@ public abstract class RedissonBaseMapIterator<K, V, M> implements Iterator<M> {
private boolean finished; private boolean finished;
private boolean currentElementRemoved; private boolean currentElementRemoved;
private boolean removeExecuted;
protected Map.Entry<ScanObjectEntry, ScanObjectEntry> entry; protected Map.Entry<ScanObjectEntry, ScanObjectEntry> entry;
@Override @Override
@ -49,7 +56,6 @@ public abstract class RedissonBaseMapIterator<K, V, M> implements Iterator<M> {
free(lastValues); free(lastValues);
currentElementRemoved = false; currentElementRemoved = false;
removeExecuted = false;
client = null; client = null;
firstValues = null; firstValues = null;
lastValues = null; lastValues = null;
@ -60,15 +66,15 @@ public abstract class RedissonBaseMapIterator<K, V, M> implements Iterator<M> {
} }
finished = false; finished = false;
} }
long prevIterPos;
do { do {
prevIterPos = nextIterPos;
MapScanResult<ScanObjectEntry, ScanObjectEntry> res = iterator(); MapScanResult<ScanObjectEntry, ScanObjectEntry> res = iterator();
if (lastValues != null) { if (lastValues != null) {
free(lastValues); free(lastValues);
} }
lastValues = convert(res.getMap()); lastValues = convert(res.getMap());
client = res.getRedisClient(); client = res.getRedisClient();
if (nextIterPos == 0 && firstValues == null) { if (nextIterPos == 0 && firstValues == null) {
firstValues = lastValues; firstValues = lastValues;
lastValues = null; lastValues = null;
@ -76,7 +82,6 @@ public abstract class RedissonBaseMapIterator<K, V, M> implements Iterator<M> {
client = null; client = null;
firstValues = null; firstValues = null;
nextIterPos = 0; nextIterPos = 0;
prevIterPos = -1;
} }
} else { } else {
if (firstValues.isEmpty()) { if (firstValues.isEmpty()) {
@ -87,38 +92,38 @@ public abstract class RedissonBaseMapIterator<K, V, M> implements Iterator<M> {
client = null; client = null;
firstValues = null; firstValues = null;
nextIterPos = 0; nextIterPos = 0;
prevIterPos = -1;
continue; continue;
} }
if (res.getPos() == 0) { if (res.getPos() == 0) {
free(firstValues);
free(lastValues);
finished = true; finished = true;
return false; return false;
} }
} }
} else if (lastValues.keySet().removeAll(firstValues.keySet())) { } else if (lastValues.keySet().removeAll(firstValues.keySet())
|| (lastValues.isEmpty() && nextIterPos == 0)) {
free(firstValues); free(firstValues);
free(lastValues); free(lastValues);
currentElementRemoved = false; currentElementRemoved = false;
removeExecuted = false;
client = null; client = null;
firstValues = null; firstValues = null;
lastValues = null; lastValues = null;
nextIterPos = 0; nextIterPos = 0;
prevIterPos = -1;
if (tryAgain()) { if (tryAgain()) {
continue; continue;
} }
finished = true; finished = true;
return false; return false;
} }
} }
lastIter = res.getMap().entrySet().iterator(); lastIter = res.getMap().entrySet().iterator();
nextIterPos = res.getPos(); nextIterPos = res.getPos();
} while (!lastIter.hasNext() && nextIterPos != prevIterPos); } while (!lastIter.hasNext());
if (prevIterPos == nextIterPos && !removeExecuted) {
finished = true;
}
} }
return lastIter.hasNext(); return lastIter.hasNext();
@ -184,7 +189,6 @@ public abstract class RedissonBaseMapIterator<K, V, M> implements Iterator<M> {
lastIter.remove(); lastIter.remove();
removeKey(); removeKey();
currentElementRemoved = true; currentElementRemoved = true;
removeExecuted = true;
entry = null; entry = null;
} }

@ -66,6 +66,7 @@ public class RedissonKeysTest extends BaseTest {
for (int i = 0; i < 115; i++) { for (int i = 0; i < 115; i++) {
String key = "key" + Math.random(); String key = "key" + Math.random();
RBucket<String> bucket = redisson.getBucket(key); RBucket<String> bucket = redisson.getBucket(key);
keys.add(key);
bucket.set("someValue"); bucket.set("someValue");
} }

@ -6,6 +6,7 @@ import static org.redisson.BaseTest.createInstance;
import java.io.IOException; import java.io.IOException;
import java.net.InetSocketAddress; import java.net.InetSocketAddress;
import java.util.Arrays;
import java.util.Collections; import java.util.Collections;
import java.util.Iterator; import java.util.Iterator;
import java.util.Map; import java.util.Map;
@ -33,10 +34,13 @@ import org.redisson.client.RedisConnectionException;
import org.redisson.client.RedisException; import org.redisson.client.RedisException;
import org.redisson.client.RedisOutOfMemoryException; import org.redisson.client.RedisOutOfMemoryException;
import org.redisson.client.protocol.decoder.ListScanResult; import org.redisson.client.protocol.decoder.ListScanResult;
import org.redisson.client.protocol.decoder.ScanObjectEntry;
import org.redisson.codec.SerializationCodec; import org.redisson.codec.SerializationCodec;
import org.redisson.config.Config; import org.redisson.config.Config;
import org.redisson.connection.ConnectionListener; import org.redisson.connection.ConnectionListener;
import io.netty.buffer.Unpooled;
public class RedissonTest { public class RedissonTest {
protected RedissonClient redisson; protected RedissonClient redisson;
@ -76,7 +80,7 @@ public class RedissonTest {
} }
@Test @Test
public void testIterator() { public void testIteratorNotLooped() {
RedissonBaseIterator iter = new RedissonBaseIterator() { RedissonBaseIterator iter = new RedissonBaseIterator() {
int i; int i;
@Override @Override
@ -101,6 +105,41 @@ public class RedissonTest {
Assert.assertFalse(iter.hasNext()); Assert.assertFalse(iter.hasNext());
} }
@Test
public void testIteratorNotLooped2() {
RedissonBaseIterator<Integer> iter = new RedissonBaseIterator<Integer>() {
int i;
@Override
ListScanResult<ScanObjectEntry> iterator(InetSocketAddress client, long nextIterPos) {
i++;
if (i == 1) {
return new ListScanResult<ScanObjectEntry>(14L, Arrays.asList(new ScanObjectEntry(Unpooled.wrappedBuffer(new byte[] {1}), 1)));
}
if (i == 2) {
return new ListScanResult(7L, Collections.emptyList());
}
if (i == 3) {
return new ListScanResult(0L, Collections.emptyList());
}
if (i == 4) {
return new ListScanResult(14L, Collections.emptyList());
}
Assert.fail();
return null;
}
@Override
void remove(Integer value) {
}
};
Assert.assertTrue(iter.hasNext());
assertThat(iter.next()).isEqualTo(1);
Assert.assertFalse(iter.hasNext());
}
@BeforeClass @BeforeClass
public static void beforeClass() throws IOException, InterruptedException { public static void beforeClass() throws IOException, InterruptedException {
if (!RedissonRuntimeEnvironment.isTravis) { if (!RedissonRuntimeEnvironment.isTravis) {
@ -250,7 +289,7 @@ public class RedissonTest {
Assert.assertEquals(0, pp.stop()); Assert.assertEquals(0, pp.stop());
await().atMost(1, TimeUnit.SECONDS).until(() -> assertThat(connectCounter.get()).isEqualTo(1)); await().atMost(2, TimeUnit.SECONDS).until(() -> assertThat(connectCounter.get()).isEqualTo(1));
await().until(() -> assertThat(disconnectCounter.get()).isEqualTo(1)); await().until(() -> assertThat(disconnectCounter.get()).isEqualTo(1));
} }

Loading…
Cancel
Save