SimpleAsyncTaskExecutor not respect ConcurrencyThrottleSupport.NO_CONCURRENCY limit [SPR-15895]

[spring-projects-issues]

Simon Wong opened SPR-15895 and commented

When I set the SimpleAsyncTaskExecutor setConcurrencyLimit with value ConcurrencyThrottleSupport.NO_CONCURRENCY (i.e. value = 0), the execute of SimpleAsyncTaskExecutor will not take NO_CONCURRENCY in consideration. The Runnable will always be executed in all cases.

public void execute(Runnable task, long startTimeout) {
     Assert.notNull(task, "Runnable must not be null");
     Runnable taskToUse = (this.taskDecorator != null ? this.taskDecorator.decorate(task) : task);
     if (isThrottleActive() && startTimeout > TIMEOUT_IMMEDIATE) {
          this.concurrencyThrottle.beforeAccess();
          doExecute(new ConcurrencyThrottlingRunnable(taskToUse));
     }
     else {
          doExecute(taskToUse);
     }
}

The root cause is isThrottleActive() will always returns false as the concurrent limit = 0 in this case, hence ConcurrencyThrottleAdapter will not be processed.

This is also the reason why the 1st test case SimpleAsyncTaskExecutorTests cannotExecuteWhenConcurrencyIsSwitchedOff is disabled by the SPF committer.

The current workaround is SimpleAsyncTaskExecutor.setConcurrencyLimit(1).

BTW, NO_CONCURRENCY = 0 is somehow wrong in definition. No currency should means there should be one invocation of method at a time, other invocations should be waited. That means NO_CONCURRENCY should have a value of 1 instead.

---

Affects: 4.3.10

Issue Links:

• Migrate remaining JUnit 3 based tests to JUnit 4 [SPR-13514] #18091 Migrate remaining JUnit 3 based tests to JUnit 4

Referenced from: commits f3a2f57, 204ddeb

[spring-projects-issues]

Juergen Hoeller commented

NO_CONCURRENCY - as originally defined in ConcurrencyThrottleSupport - actually means no entrance to the monitor at all, so in this case no async execution at all. In that sense, 0 is the right value for it... but there is nevertheless a bug in that SimpleAsyncTaskExecutor doesn't respect that due to isThrottleActive() returning false in that case. Fixed for 5.0 RC4 now, and to be backported to 4.3.11.

For your purposes, setting the concurrency limit to 1 is indeed the right solution.

[spring-projects-issues]

Simon Wong commented

Other than just change the value for NO_CONCURRENCY from 0 to 1, will it be better to introduce a new constant,say, NO_ACCESS with value 0, to respect the original semantics (i.e. not entrance at all)? In that case, I could set the concurrency value at runtime (e.g. with JMX) to disable the access of the operation and it will throw some kinds of exception if access it.

[spring-projects-issues]

Juergen Hoeller commented

To be clear, the value of the constant hasn't changed here, and I do not intend to change it. The semantics of the existing constants are established for other ConcurrencyThrottleSupport usage scenarios, so the only thing we can really do is to tighten the definition there for the no-access case that it actually represents. From the perspective of the submitting thread, this doesn't seem so wrong either: There won't be any concurrent execution (no async tasks) next to the original thread.

For your scenario, a custom concurrencyLimit value of 1 seems quite expressive, so I'm not sure there needs to be a dedicated constant for this in the first place. After all, the constants are just named placeholders for use in Java code... but the real values are the underlying integers anyway. For a JMX scenario, the values sound plausible enough as they are now: simply using -1 for unbounded, 0 for no access at all, and 1 for a maximum of one async task execution at the same time.

So the actual fix here is just that NO_CONCURRENCY / 0 actually works now since isThrottleActive() returns true for it. Instead of just letting every task through like in unbounded mode, SimpleAsyncTaskExecutor will reject any task in no-concurrency now mode, as it was originally meant to do.