elsa-workflows / elsa-workflows/elsa-core
[BUG] ParallelForEach Behavior Inconsistency
- Dominant language
- C#
- Stars
- 7.9k
- Forks
- 1.5k
- Avg merge
- 15h 22m
- Merged PRs (30d)
- 114
Description
**Description:**
In the Elsa Workflow , I believe there's a potential issue with the `ParallelForEach` implementation. After examining the code, it seems that the usage of `CurrentValue` variable within the loop may lead to race conditions and incorrect behavior in multi-threaded scenarios.
**Code Segment:**
```csharp
context.SetProperty(ScheduledTagsProperty, tags);
context.SetProperty(CompletedTagsProperty, new List());
await foreach (var item in items)
{
// For each item, declare a new variable for the work to be scheduled.
var currentValueVariable = new Variable("CurrentValue", item)
{
// TODO: This should be configurable, because this won't work for e.g. file streams and other non-serializable types.
StorageDriverType = typeof(WorkflowStorageDriver)
};
var currentIndexVariable = new Variable("CurrentIndex", currentIndex++) { StorageDriverType = typeof(WorkflowStorageDriver) };
var variables = new List { currentValueVariable, currentIndexVariable };
// Schedule a body of work for each item.
var tag = Guid.NewGuid();
tags.Add(tag);
await context.ScheduleActivityAsync(Body, OnChildCompleted, tag, variables);
}
```
**Issue:**
The concern arises from the fact that the `CurrentValue` variable is shared across iterations of the loop. Since each iteration schedules work asynchronously, there's a possibility of race conditions where one thread might modify the `CurrentValue` before another thread can consume its value.
**Expected Behavior:**
In a parallel loop like this, each iteration should have its own isolated context, including its own instance of the `CurrentValue` variable. This ensures that modifications made by one iteration do not affect the others.
Contributor guide
Assessment
This issue has not been assessed yet.