https://github.com/antirez/redis/blob/69ce235c7b870e43813da7e03160c320389ea6a9/src/ziplist.c#L783
reqlen include prevlen ，and prevlen equal to p's prevlen.
In case of nextdiff == -4, the p's prevlen is 5 bytes，and the prevlen for reqlen is 1byte.
How this happen, as i see, reqlen > prevlen == p's prevlen
Comment From: zuiderkwast
Some context: This is in the __ziplistInsert function.
/* When the insert position is not equal to the tail, we need to
* make sure that the next entry can hold this entry's length in
* its prevlen field. */
int forcelarge = 0;
nextdiff = (p[0] != ZIP_END) ? zipPrevLenByteDiff(p,reqlen) : 0;
if (nextdiff == -4 && reqlen < 4) {
nextdiff = 0;
forcelarge = 1;
}
zipPrevLenByteDiff(a, b) returns bytesize(b) - bytesize(a). When nextdiff == -4, it means that p (the next element) had prevlen 5 before and now only needs prevlen 1.
I guess it could only happen if this function would replace one element with another, but this is __ziplistInsert, so I agree, I think it cannot happen. Maybe the if-statement can be removed.
Comment From: zuiderkwast
I checked code coverage using make lcov and I can see this if-statement is never reached when running the tests. I think it is dead code and should be deleted.
Comment From: enjoy-binbin
It is actually useful. There will be problems during chain update, but it's hard to explain. I can try to add test cases to this and prompt others to see this (WDYT @madolson @zuiderkwast ). It took me a while to understand it and reproduce it...
So let's comment out these two lines first
nextdiff = 0;
forcelarge = 1;
Then follow the steps:
127.0.0.1:6379> flushdb
OK
127.0.0.1:6379> rpush list one
(integer) 1
127.0.0.1:6379> rpush list two
(integer) 2
127.0.0.1:6379> rpush list AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
(integer) 3
127.0.0.1:6379> rpush list AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
(integer) 4
127.0.0.1:6379> rpush list three
(integer) 5
127.0.0.1:6379> rpush list 10
(integer) 6
127.0.0.1:6379> lrem list 1 AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
(integer) 1
127.0.0.1:6379> linsert list after AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA 10
(integer) 6
127.0.0.1:6379> lrange list 0 -1
1) "one"
2) "two"
3) "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"
4) "10"
5) "three"
6) "-123456789"
We can see the last element. 10 became -123456789.
And then we do a save, try to get a rdb file,
and uncoment nextdiff = 0; and forcelarge = 1;
and remake redis and start the server
we can see rdb file was broken..
18032:M 03 Jul 2021 16:42:31.746 * RDB memory usage when created 0.88 Mb
18032:M 03 Jul 2021 16:42:31.746 # Internal error in RDB reading offset 0, function at rdb.c:1802 -> Ziplist integrity check failed.
[offset 0] Checking RDB file dump.rdb
[offset 32] AUX FIELD redis-ver = '255.255.255'
[offset 46] AUX FIELD redis-bits = '64'
[offset 58] AUX FIELD ctime = '1625301742'
[offset 73] AUX FIELD used-mem = '918774'
[offset 89] AUX FIELD aof-preamble = '0'
[offset 91] Selecting DB ID 0
--- RDB ERROR DETECTED ---
[offset 146] Internal error in RDB reading offset 0, function at rdb.c:1802 -> Ziplist integrity check failed.
[additional info] While doing: read-object-value
[additional info] Reading key 'list'
[additional info] Reading type 14 (quicklist)
[info] 1 keys read
[info] 0 expires
[info] 0 already expired
18032:M 03 Jul 2021 16:42:31.746 # Terminating server after rdb file reading failure.
Then i delete the rdb file, restart the server, re-execute the previous steps
127.0.0.1:6379> flushdb
OK
127.0.0.1:6379> rpush list one
(integer) 1
127.0.0.1:6379> rpush list two
(integer) 2
127.0.0.1:6379> rpush list AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
(integer) 3
127.0.0.1:6379> rpush list AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
(integer) 4
127.0.0.1:6379> rpush list three
(integer) 5
127.0.0.1:6379> rpush list 10
(integer) 6
127.0.0.1:6379> lrem list 1 AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
(integer) 1
127.0.0.1:6379> linsert list after AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA 10
(integer) 6
127.0.0.1:6379> lrange list 0 -1
1) "one"
2) "two"
3) "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"
4) "10"
5) "three"
6) "10"
The result is ok. This situation will not cause crash. But it will cause data corruption and break the rdb file. There are more demanding conditions that may cause crash.
A crash example:
127.0.0.1:6379> flushdb
OK
127.0.0.1:6379> rpush list one
(integer) 1
127.0.0.1:6379> rpush list two
(integer) 2
127.0.0.1:6379> rpush list AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
(integer) 3
127.0.0.1:6379> rpush list AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
(integer) 4
127.0.0.1:6379> rpush list three
(integer) 5
127.0.0.1:6379> rpush list b
(integer) 6
127.0.0.1:6379> lrem list 1 AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA
(integer) 1
127.0.0.1:6379> linsert list after AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA 10
Could not connect to Redis at 127.0.0.1:6379: Connection refused
not connected>
will crash on assert(zipEntrySafe(zl, curlen, p, &cur, 0));. serverlog:
=== REDIS BUG REPORT START: Cut & paste starting from here ===
21658:M 03 Jul 2021 17:01:53.997 # === ASSERTION FAILED ===
21658:M 03 Jul 2021 17:01:53.997 # ==> ziplist.c:776 'zipEntrySafe(zl, curlen, p, &cur, 0)' is not true
------ STACK TRACE ------
Backtrace:
src/redis-server 127.0.0.1:6379(__ziplistCascadeUpdate+0x86c)[0x44d3cc]
src/redis-server 127.0.0.1:6379(__ziplistInsert+0x339)[0x4528b9]
src/redis-server 127.0.0.1:6379(ziplistInsert+0x11)[0x454451]
src/redis-server 127.0.0.1:6379(_quicklistInsert+0xc2)[0x42d802]
src/redis-server 127.0.0.1:6379(quicklistInsertAfter+0x17)[0x42de57]
src/redis-server 127.0.0.1:6379(listTypeInsert+0xad)[0x48bacd]
src/redis-server 127.0.0.1:6379(linsertCommand+0x193)[0x48c283]
src/redis-server 127.0.0.1:6379(call+0xaa)[0x4392ca]
src/redis-server 127.0.0.1:6379(processCommand+0xbe5)[0x43bd75]
src/redis-server 127.0.0.1:6379(processCommandAndResetClient+0x24)[0x459fc4]
src/redis-server 127.0.0.1:6379(processInputBuffer+0x102)[0x45dec2]
src/redis-server 127.0.0.1:6379(readQueryFromClient+0x428)[0x461c98]
src/redis-server 127.0.0.1:6379[0x52f786]
src/redis-server 127.0.0.1:6379(aeProcessEvents+0x3b9)[0x42f259]
src/redis-server 127.0.0.1:6379(aeMain+0x2d)[0x42f79d]
src/redis-server 127.0.0.1:6379(main+0x503)[0x429aa3]
/lib64/libc.so.6(__libc_start_main+0xf5)[0x7f5a27d273d5]
src/redis-server 127.0.0.1:6379[0x42a793]
...
id=3 addr=127.0.0.1:35898 laddr=127.0.0.1:6379 fd=8 name= age=101 idle=0 flags=N db=0 sub=0 psub=0 multi=-1 qbuf=304 qbuf-free=16080 argv-mem=268 obl=0 oll=0 omem=0 tot-mem=33714 events=r cmd=linsert user=default redir=-1
argv[0]: 'linsert'
argv[1]: 'list'
argv[2]: 'after'
argv[3]: 'AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA'
argv[4]: '10'
22984:M 03 Jul 2021 17:09:16.961 # key 'list' found in DB containing the following object:
22984:M 03 Jul 2021 17:09:16.961 # Object type: 1
22984:M 03 Jul 2021 17:09:16.961 # Object encoding: 9
22984:M 03 Jul 2021 17:09:16.961 # Object refcount: 1
Comment From: sundb
This is explained in https://github.com/redis/redis/commit/c495d095ae495ea5253443ee4562aaa30681a854.
Comment From: oranagra
@enjoy-binbin I didn't follow the steps closely, where you able to get to that assertion without modifying redis? or was it a result of loading an RDB file that was produced by the modified code?
Comment From: enjoy-binbin
@oranagra oh my bad. The crash was caused when I commented out those two lines of code.
nextdiff = 0; forcelarge = 1;
I want to show that the if judgment is useful.
Comment From: oranagra
@enjoy-binbin i would have suggested to add the the pattern you just shown to the test suite, but considering that ziplist is soon going to be dismissed (see #8887), maybe it doesn't worth the effort. another idea is maybe add this case to the tests at the bottom of ziplist.c (or maybe the tests there already cover that case).
Comment From: enjoy-binbin
It’s no problem for me. I will try to add it both if conditions permit. I think it’s not bad that this situation can be recorded. improve the test case