Skip to content

Fix pooled connection leak from unclosed pipelines and transactions - #334

Open
ali-jawad wants to merge 2 commits into
gresrun:masterfrom
ali-jawad:close-batch-enqueue-pipeline
Open

ali-jawad wants to merge 2 commits into
gresrun:masterfrom
ali-jawad:close-batch-enqueue-pipeline

Conversation

@ali-jawad

Copy link
Copy Markdown

Fixes #333

Since 3.0.0, ClientPoolImpl.batchEnqueue() leaks one pooled connection per call. UnifiedJedis.pipelined() borrows a connection that only Pipeline.close() returns, but AbstractClient.doBatchEnqueue() only calls sync(). With a bounded pool, the call after maxTotal fails with JedisException: Could not get a resource from the pool. With the default unbounded pool config, each call leaves a connection open on Redis.

UnifiedJedis.multi() has the same contract, so the same leak affects:

  • ClientPoolImpl.recurringEnqueue() and removeRecurringEnqueue();
  • WorkerPoolImpl failure handling when a FailQueueStrategy limits the fail queue's max items. There the exception is caught and logged, so failures silently stop being recorded.

2.x was unaffected because these calls ran inside PoolUtils.doWorkInPool().

  • Close the pipeline and the transactions with try-with-resources. For ClientImpl/WorkerImpl (Jedis.pipelined()/multi()), closing leaves the connection open.
  • Add regression tests that run more operations than a bounded pool's maxTotal. They fail on master and pass with this change.

UnifiedJedis.pipelined() borrows a connection from the pool that is only
returned when the pipeline is closed, so every ClientPoolImpl.batchEnqueue()
call leaked one connection until the pool was exhausted.
UnifiedJedis.multi() also borrows a connection from the pool that is only
returned when the transaction is closed, so ClientPoolImpl's recurring
enqueue/removal and WorkerPoolImpl's fail queue trimming (when a
FailQueueStrategy limits the max items) leaked one connection per call.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Pooled clients leak a connection per call: pipelines and transactions taken from UnifiedJedis are never closed

1 participant