graphql-java / graphql-java/java-dataloader

[Bug] Memory Leak and Stats Pollution via unmanaged ThreadLocal in ThreadLocalStatisticsCollector

Open
#266 1 comment 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

keep-open
Dominant language
Java
Stars
525
Forks
101
PR merge metrics
No merged PRs in 30d

Description

Describe the bug
There is a potential Unintentional ThreadLocal Leak (UTL) in ThreadLocalStatisticsCollector. The class relies on consumers manually calling resetThread() at "request boundaries" to clear the ThreadLocal<SimpleStatisticsCollector>.

However, in typical GraphQL environments (reactive streams, async gateways, or standard thread pools), threads are heavily reused. If an unhandled exception occurs or a developer forgets to call resetThread(), the SimpleStatisticsCollector object is permanently retained by the worker thread.

Impact:

  1. Data Pollution: Subsequent requests processed by the dirty thread will inherit the accumulated statistics of previous requests, leading to completely distorted metrics.
  2. Memory Leak (Type I UTL): As more threads in the pool retain these un-cleared statistics objects over time, the heap usage will grow linearly, potentially leading to an OutOfMemoryError in high-throughput applications.

To Reproduce
Here is a simplified code example demonstrating the data pollution in a thread-pool environment when resetThread() is missed (e.g., bypassed due to an exception):

import org.dataloader.stats.ThreadLocalStatisticsCollector;
import java.util.concurrent.*;

public class UTLReproduction {
    public static void main(String[] args) throws Exception {
        ThreadLocalStatisticsCollector collector = new ThreadLocalStatisticsCollector();
        // Simulate a web server with a reusable thread pool
        ExecutorService threadPool = Executors.newFixedThreadPool(1);

        // Request 1: Execution completes but resetThread() is missed (e.g. exception thrown)
        threadPool.submit(() -> {
            collector.incrementLoadCount();
            // Developer forgets to put collector.resetThread() in a finally block
        }).get();

        // Request 2: A new incoming request reuses the same dirty thread
        threadPool.submit(() -> {
            long loadCount = collector.getStatistics().getLoadCount();
            // BUG: loadCount is 1 instead of 0! The new request is polluted by Request 1.
            System.out.println("New Request Load Count (Expected 0): " + loadCount);
        }).get();
        
        threadPool.shutdown();
    }
}

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reading ThreadLocalStatisticsCollector, especially its ThreadLocal usage, getStatistics(), and resetThread(). Run the supplied fixed-thread-pool reproduction to verify whether statistics persist between requests when resetThread() is skipped. Done should include a decided way to prevent cross-request pollution and retention, with tests covering the reproduced scenario.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
backend, observability
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.