[Fix #1670] Stopping scheduledexecutorservice before clearing listeners

[fjtirado]

Fix #1670

[Copilot]

🟡 Changes recommended

One or more issues must be addressed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR reorders application cleanup to stop scheduled execution before listeners are cleared, addressing shutdown-time concurrent modification errors.

Changes:

• Adds shared executor shutdown logic.
• Shuts down the scheduler before clearing listeners.
• Reuses shutdown logic for the default executor factory.

File summaries

File	Description
impl/core/src/main/java/io/serverlessworkflow/impl/WorkflowUtils.java	Updated as part of this pull request.
impl/core/src/main/java/io/serverlessworkflow/impl/WorkflowApplication.java	Updated as part of this pull request.
impl/core/src/main/java/io/serverlessworkflow/impl/DefaultExecutorServiceFactory.java	Updated as part of this pull request.

Review details

• Files reviewed: 3/3 changed files
• Comments generated: 4
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Copilot]

🔵 Needs a closer look

One or more issues must be addressed before approval.

Review details

Suppressed comments (3)

impl/core/src/main/java/io/serverlessworkflow/impl/DefaultExecutorServiceFactory.java:31

• safeShutdown uses shutdownNow(), so this change turns the factory's previous graceful shutdown() into an interrupting shutdown. This executor runs InMemoryEvents.publish and fork branches; closing the application can now abort in-flight work or discard queued work instead of allowing it to finish within the existing two-second wait. Keep this factory on a graceful shutdown path and use the forceful path only for the scheduler.

    WorkflowUtils.safeShutdown(service);

impl/core/src/main/java/io/serverlessworkflow/impl/WorkflowApplication.java:589

• ScheduledInstanceRunnable submits work through application.executorService() after running on this scheduler. Closing executorFactory first leaves a race where a scheduled callback reaches execute after that pool has been shut down, producing RejectedExecutionException; stop and await the scheduler before closing the executor factory.

    safeShutdown(schedulerExecutorService);

impl/core/src/main/java/io/serverlessworkflow/impl/WorkflowUtils.java:183

• shutdownNow() returns only tasks removed from the queue, not tasks already running. When the service has an active task but no queued tasks—which is common for this cached pool—this condition skips awaitTermination, so close() can clear listeners while the task is still publishing events. Always await after calling shutdownNow() (subject to the timeout).

    if (service != null && !service.isShutdown() && !service.shutdownNow().isEmpty()) {

• Files reviewed: 3/3 changed files
• Comments generated: 0 new
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

One or more issues must be addressed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

impl/core/src/main/java/io/serverlessworkflow/impl/DefaultExecutorServiceFactory.java:31

• This changes the default application executor from the previous graceful shutdown() to shutdownNow(). That executor is also used for in-flight workflow execution and, when the default in-memory event consumer is created, event delivery; interrupting it can abort those operations rather than merely stopping future work. Keep the factory's graceful shutdown semantics and reserve the forceful helper for the scheduler, or make this policy explicit.

    WorkflowUtils.safeShutdown(service);

• Files reviewed: 3/3 changed files
• Comments generated: 2
• Review effort level: Lite

[Copilot]

🟡 Changes recommended

One or more issues must be addressed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

impl/core/src/main/java/io/serverlessworkflow/impl/WorkflowApplication.java:589

• The shared executor is shut down on the preceding line while the scheduled executor is still accepting work. A scheduled task can race this call and ScheduledInstanceRunnable then submits the workflow through application().executorService().execute(...), which now throws RejectedExecutionException. The close sequence needs to stop/drain scheduled submissions and coordinate completion callbacks before shutting down the shared executor.

    safeShutdown(schedulerExecutorService);

• Files reviewed: 3/3 changed files
• Comments generated: 1
• Review effort level: Lite

[gmunozfe]

Looks good to me, well done @fjtirado