[Fix #1354] ForExecutor was not properly implemented for multithread#1369
[Fix #1354] ForExecutor was not properly implemented for multithread#1369fjtirado wants to merge 1 commit intoserverlessworkflow:mainfrom
Conversation
fjtirado
force-pushed
the
Fix_1363
branch
2 times, most recently
from
e30fae0 to
5f1e7df
Compare
fjtirado
changed the title
There was a problem hiding this comment.
Pull request overview
This PR addresses a concurrency/correctness issue in the ForExecutor loop execution, ensuring loop variables are set per-iteration at execution time (rather than being overwritten while building the future chain), and updates an existing regression test to better reproduce timing-sensitive behavior.
Changes:
- Refactor
ForExecutorto execute the loop via a per-iteration future chain (instead of pre-building the whole chain in a tightwhileloop). - Add a lagged in-memory event broker for tests to introduce a small delay after publishing.
- Update
ForEachFuncTestto use the lagged broker to reproduce the bug scenario more reliably.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| impl/core/src/main/java/io/serverlessworkflow/impl/executors/ForExecutor.java | Refactors for-loop execution to avoid loop-variable overwrites across async stages. |
| experimental/test/src/test/java/io/serverlessworkflow/fluent/test/LaggedInMemoryEvent.java | Adds a test-only event broker that delays after publish to amplify timing issues. |
| experimental/test/src/test/java/io/serverlessworkflow/fluent/test/ForEachFuncTest.java | Switches the regression test to use the lagged broker. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…d for multithread Signed-off-by: fjtirado <ftirados@redhat.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Fix #1354