RedisLabs / redis-cluster-proxy

A proxy for Redis clusters.
GNU Affero General Public License v3.0
990 stars 129 forks source link

Potential memory and connection leak in sendMessageToThread #62

Open HuangZhenLiang opened 4 years ago

HuangZhenLiang commented 4 years ago

For now, if pipe is not yet writable, function sendMessageToThread will register a write event and continue sending message in callback handlePendingAwakeMessages. Function handlePendingAwakeMessages will again call sendMessageToThread to do the work. This might cause some issues.

1, If error occurs in sendMessageToThread called by handlePendingAwakeMessages, object client being sent will not be freed, memory and connection will not be freed;

2, sds msg = ln->value; int sent = sendMessageToThread(thread, msg); if (sent == -1) continue; else { listDelNode(thread->pending_messages, ln); if (!sent) { proxyLogErr("Failed to send message to thread %d", thread->thread_id); } } In function handlePendingAwakeMessages, it will not delete msg from pending_messages list if sendMessageToThread return -1. But actually in case returning -1, sendMessageToThread already add remaining buf to the tail of pending_messages list. So we have duplicate messages in the list pointing to the same content.

3, In some situations, multiple msgs being sent by sendMessageToThread might cross sent