spring-cloud / spring-cloud-deployer-local

The Spring Cloud Deployer implementation for a "local" machine
Apache License 2.0
39 stars 53 forks source link

maxConcurrentExecutionsReached() has a race-condition where multiple instances can start before it returns a result #219

Closed ccanning closed 1 year ago

ccanning commented 1 year ago

In the launch method of LocalTaskLauncher, the

if (this.maxConcurrentExecutionsReached()) {
            throw new IllegalStateException(
                String.format("Cannot launch task %s. The maximum concurrent task executions is at its limit [%d].",
                    request.getDefinition().getName(), this.getMaximumConcurrentTasks())
            );
        }

check isn't synchronized, and multiple instances can start before this code returns

public int getRunningTaskExecutionCount() {
        int runningExecutionCount = 0;

        for (TaskInstance taskInstance: running.values()) {
            if (taskInstance.getProcess().isAlive()) {
                runningExecutionCount++;
            }
        }
        return runningExecutionCount;
    }

an easy test is to set the max instances to 1 and kick off multiple concurrent requests and have it start docker tasks.

From investigation, the other launchers handle this differently and don't seem to have this issue.