elsa-workflows / elsa-workflows/elsa-core

Duplicate workflow instances are created for the same CorrelationId

Open
#7,948 0 comments 0 reactions 0 assignees View on GitHub
bug core triaged
Dominant language
C#
Stars
7.9k
Forks
1.5k
Avg merge
15h 22m
Merged PRs (30d)
114

Description

**Elsa version:** 3.7
**Persistence:** EF Core (SQL Server)
**Runtime:** default (`UseDefaultRuntime`)

## Description

Dispatching a workflow twice with the same `CorrelationId` creates two running instances. Nothing in
the dispatch path checks whether an instance already exists for that correlation id, and the database
does not prevent it.

This makes `CorrelationId` unusable as the identity of a long-running conversation, which is the
scenario it appears designed for: a webhook receives events, each event carries a correlation id, the
first event starts a workflow and later events resume it.

## Expected behaviour

Dispatching with a `CorrelationId` that already has a running instance either resumes/attaches to that
instance, or fails — but does not silently create a second one.

## Actual behaviour

Two running instances exist with the same `CorrelationId`. Every subsequent lookup returns both, and
there is no way to tell which one holds the conversation's state.

## Steps to reproduce

1. `POST` two webhook events carrying the same correlation id, close enough together that the first
instance is not yet persisted when the second is handled.
2. Each handler queries for a running instance, finds none, and calls
`IWorkflowDispatcher.DispatchAsync(new DispatchWorkflowDefinitionRequest { CorrelationId = "abc", ... })`.
3. Query `WorkflowInstanceFilter { CorrelationId = "abc", WorkflowStatus = Running }`.

**Result:** two instances. **Expected:** one.

The window is small, but a webhook under load hits it routinely, and it is permanently open for any
deployment running more than one node behind a load balancer.

## Where this happens in the code

**1. The column is indexed but not unique** — `src/modules/Elsa.Persistence.EFCore/Modules/Management/Configurations.cs`:

```csharp
builder.HasIndex(x => x.CorrelationId)
.HasDatabaseName($"IX_{nameof(WorkflowInstance)}_{nameof(WorkflowInstance.CorrelationId)}");
```

Elsa does use `.IsUnique()` elsewhere in the same schema (`StoredTrigger` on
`WorkflowDefinitionId, Hash, ActivityId`), so the omission here is load-bearing.

**2. The dispatcher passes the correlation id straight through without consulting it** —
`src/modules/Elsa.Workflows.Runtime/Services/BackgroundWorkflowDispatcher.cs`:

```csharp
var command = new DispatchWorkflowDefinitionCommand(request.DefinitionVersionId)
{
Input = request.Input,
Properties = request.Properties,
CorrelationId = request.CorrelationId,
InstanceId = request.InstanceId,
TriggerActivityId = request.TriggerActivityId,
...
};
```

**3. Instance creation never looks for an existing instance with that correlation id** —
`src/modules/Elsa.Workflows.Runtime/Services/LocalWorkflowClient.cs`:

```csharp
public async Task CreateInstanceInternalAsync(CreateWorkflowInstanceRequest request, CancellationToken cancellationToken = default)
{
var workflowDefinitionHandle = request.WorkflowDefinitionHandle;
var workflowGraph = await GetWorkflowGraphAsync(workflowDefinitionHandle, cancellationToken);

var options = new WorkflowInstanceOptions
{
WorkflowInstanceId = WorkflowInstanceId,
CorrelationId = request.CorrelationId,
Name = request.Name,
ParentWorkflowInstanceId = request.ParentId,
Input = request.Input,
Properties = request.Properties
};

return workflowInstanceManager.CreateWorkflowInstance(workflowGraph.Workflow, options);
}
```

The correlation id is carried as data only. No read, no constraint, no collision.

## Impact

Once duplicates exist, application state keyed to "the instance for this correlation" is ambiguous, and
cleaning up is destructive — deleting the "extra" instance can delete the one that actually holds the
conversation's state, because nothing distinguishes them.

Working around it outside the framework is unsatisfying: checking before dispatching is check-then-act
and does not close the race; serialising per correlation id works only within one process; a
distributed lock guards the caller's check rather than anything the persistence layer enforces.

## Suggested fix

Either:

- a unique filtered index on `CorrelationId` for non-terminal statuses, so a second create collides
rather than succeeding; or
- an opt-in on `DispatchWorkflowDefinitionRequest` — e.g. `SingleInstancePerCorrelationId` — enforced
where the instance is created rather than by the caller.

If the current behaviour is intentional and correlation is only ever meant to *group* instances, that
is worth stating explicitly in the docs, because the API shape invites the opposite assumption.

Contributor guide

Open the contributing guide

Research direction

Start by tracing instance creation and persistence through src/modules/Elsa.Workflows.Runtime/Services/BackgroundWorkflowDispatcher.cs, src/modules/Elsa.Workflows.Runtime/Services/LocalWorkflowClient.cs, and src/modules/Elsa.Persistence.EFCore/Modules/Management/Configurations.cs. Reproduce concurrent dispatches with one CorrelationId, then verify the chosen behavior prevents ambiguous running instances without breaking normal dispatch.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp, sql
Domain
backend, databases, distributed-systems
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.