Skip to content

Conversation

@NYgomets
Copy link
Contributor

Problem

When concurrency limiting is enabled via setConcurrencyLimit(), if thread creation fails in doExecute() (for example, OutOfMemoryError from Thread.start()), the concurrency permit is never released, causing permanent deadlock.

Root Cause

if (isThrottleActive() && startTimeout > TIMEOUT_IMMEDIATE) {
    this.concurrencyThrottle.beforeAccess();  // (1) Increments concurrencyCount
    doExecute(new TaskTrackingRunnable(taskToUse));  // (2) May throw Error
    // If (2) throws, TaskTrackingRunnable.run() never executes
    // Therefore afterAccess() in its finally block is never called
}

Execution flow when thread creation fails:

  1. beforeAccess() increments concurrencyCount (e.g., 0 → 1)
  2. doExecute() calls newThread(task).start()
  3. Thread.start() throws OutOfMemoryError
  4. TaskTrackingRunnable object was created but run() never executes
  5. The finally block containing afterAccess() never executes
  6. concurrencyCount remains at 1 permanently

Impact

After enough failures (equal to concurrencyLimit), all subsequent task submissions permanently block:

// ConcurrencyThrottleSupport.onLimitReached()
while (this.concurrencyCount >= this.concurrencyLimit) {
    this.concurrencyCondition.await();  // Blocks forever
}

The executor becomes permanently deadlocked.

Solution

if (isThrottleActive() && startTimeout > TIMEOUT_IMMEDIATE) {
    this.concurrencyThrottle.beforeAccess();
    try {
        doExecute(new TaskTrackingRunnable(taskToUse));
    }
    catch (Throwable ex) {
        // Release permit if thread creation fails
        this.concurrencyThrottle.afterAccess();
        throw new TaskRejectedException("Failed to start execution thread", ex);
    }
}

Why Only This Path?

The else if (this.activeThreads != null) path does not need fixing:

  1. It never calls beforeAccess() (different code path)
  2. In this path, isThrottleActive() returned false, so concurrencyLimit == -1
  3. TaskTrackingRunnable.afterAccess() has guard: if (concurrencyLimit >= 0)
  4. With concurrencyLimit == -1, afterAccess() becomes no-op
  5. No double-decrement or negative count issue

Testing

The test reproduces the deadlock scenario:

  1. Set concurrencyLimit to 1
  2. Mock doExecute() to throw OutOfMemoryError
  3. First execute() call - should fail with some exception
  4. Reset mock to allow success
  5. Second execute() call - should not deadlock (the real test)

Test approach: We don't care what exception the first call throws (could be Error or TaskRejectedException). The critical test is whether the second call completes without timeout, proving the permit was released.

Before fix: Second call times out (deadlock)
After fix: Second call completes successfully

Backward Compatibility

  • No API changes
  • Behavior only changes in error scenarios (thread creation failure)
  • Normal execution flow unchanged
  • Exception wrapping (ErrorTaskRejectedException) provides better error context

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Oct 28, 2025
@NYgomets NYgomets force-pushed the fix-concurrency-deadlock branch 2 times, most recently from 6dad156 to 5d53b4e Compare October 28, 2025 22:32
…TaskExecutor

When concurrency limiting is enabled via setConcurrencyLimit() and
thread creation fails in doExecute() (e.g., OutOfMemoryError from
Thread.start()), the concurrency permit acquired by beforeAccess()
is never released because TaskTrackingRunnable.run() never executes.

This causes the concurrency count to permanently remain at the limit,
causing all subsequent task submissions to block forever in
ConcurrencyThrottleSupport.onLimitReached().

Root cause:
- beforeAccess() increments concurrencyCount
- doExecute() throws Error before thread starts
- TaskTrackingRunnable.run() never executes
- afterAccess() in finally block never called
- Concurrency permit permanently leaked

Solution:
Wrap doExecute() in try-catch block in the concurrency throttle path
and call afterAccess() in catch block to ensure permit is always
released, even when thread creation fails.

The fix only applies to the concurrency throttle path. The
activeThreads-only path does not need fixing because it never calls
beforeAccess(), so there is no permit to leak.

Test approach:
The test simulates thread creation failure and verifies that a
subsequent execution does not deadlock. The first execution should
fail with some exception (type doesn't matter), and the second
execution should complete within timeout if the permit was properly
released.

Signed-off-by: Park Juhyeong <wngud5957@naver.com>
@NYgomets NYgomets force-pushed the fix-concurrency-deadlock branch from 5d53b4e to f30f01a Compare October 28, 2025 23:25
@jhoeller jhoeller self-assigned this Oct 30, 2025
@jhoeller jhoeller added type: bug A general bug in: core Issues in core modules (aop, beans, core, context, expression) and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Oct 30, 2025
@jhoeller jhoeller added this to the 6.2.13 milestone Oct 30, 2025
@jhoeller jhoeller merged commit 14579b7 into spring-projects:6.2.x Oct 30, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

in: core Issues in core modules (aop, beans, core, context, expression) type: bug A general bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants