apache / apache/curator

[Potential Bug] Implicit creation of ExecutorService in LeaderSelector constructor risks thread leaks

Open
#1,282 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
Java
Stars
3.2k
Forks
1.2k
PR merge metrics
No merged PRs in 30d

Description

### Description
The LeaderSelector class provides a convenience constructor that implicitly creates an internal ExecutorService using Executors.newSingleThreadExecutor().

### Code Reference In org.apache.curator.framework.recipes.leader.LeaderSelector (Line 104):

```java
public LeaderSelector(CuratorFramework client, String leaderPath, LeaderSelectorListener listener) {
this(
client,
leaderPath,
// Issue: Implicitly creates a thread pool that is hidden from the user
new CloseableExecutorService(Executors.newSingleThreadExecutor(defaultThreadFactory), true),
listener);
}
```

### The Issue

1. Hidden Lifecycle: Users utilizing this constructor may not be aware that a heavy resource (Thread Pool) is created internally.

2. Thread Leak Risk: If the LeaderSelector instance is garbage collected without calling close() (e.g., during rapid Zookeeper session reconstruction or error handling flows), the internal non-daemon thread pool remains active. This leads to Thread Leaks and eventual OutOfMemoryError or thread exhaustion.

### Recommendation

Deprecate this constructor to discourage implicit resource allocation.

Or, update Javadoc to explicitly warn users that close() is mandatory to release the internal threads.

Ideally, encourage Dependency Injection where the ExecutorService is passed externally.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.