Closed rahulKrishnaM closed 2 weeks ago
I don't see that address being explicitly set to 0 after memory is freed. Do you agree with the analysis?
Posting some more thoughts,
selectNode()
is trying to do a dictNext()
. So, if we don't set the he
object to NULL once we free it inside _dictClear()
, any later dictNext()
access might still think the entry as valid and try to process that (i.e when the execution is still inside _dictClear()
).
maybe something in lines of:
while (he) {
nextHe = he->next;
+ ht->tabl[i] = nextHe;
dictFreeEntryKey(ht, he);
dictFreeEntryVal(ht, he);
hi_free(he);
ht->used--;
he = nextHe;
}
Hi, came across this crash during one of the runs.
Looks like the execution was at this line number when the crash happened
https://github.com/Nordix/hiredis-cluster/blob/a2f1b825dda8f1438bd0df93880185d1e879da99/hircluster.c#L3878
I am suspecting the reason for SEGV could be the below code area:
https://github.com/Nordix/hiredis-cluster/blob/a2f1b825dda8f1438bd0df93880185d1e879da99/dict.c#L179
where, even though we are freeing up the memory pointed by
redisClusterNode*
ordictEntry
, we are not unlinking it from the table. Hence it gets looked up later on(duringselectNode()
), which would be a dangling pointer.And also, other thing that looks odd is the library trying to reattempt discovery when the
redisClusterAsyncContext
object is getting deleted. Maybe we should prevent that path altogether, in case ofredisClusterAsyncFree()
where it's about to tear everything down anyways.cc: @bjosv