A created-but-never-started WorkflowInstance stays in activeInstances, and cancel() cannot remove it

What happened:

A WorkflowInstance that is created but never started stays in WorkflowDefinition.activeInstances for the lifetime of the JVM, and there is no supported way to discard it.

WorkflowDefinition.instance(Object) registers the instance eagerly — WorkflowMutableInstance's constructor ends with:

definition.addInstance(this);   // activeInstances.put(id(), instance)

and the entry is only ever removed by cleanUp, which runs when a run reaches a terminal state.

That is fine for startInstance(...), where creation and start are one call. It is a leak for callers that split the two, which the API invites by exposing instance(Object) separately from WorkflowInstance.start(). Any caller that decides after creating the instance that it should not run — or whose start() throws — leaks the entry.

The obvious cleanup does not work. cancel() is a no-op on a never-started instance:

// WorkflowMutableInstance.internalCancel()
if (TaskExecutorHelper.isActive(status.get())) { ... status.getAndSet(CANCELLED) ... }
else { previous = CANCELLED; }

TaskExecutorHelper.isActive(WorkflowStatus) is true only for RUNNING, WAITING and SUSPENDED. A freshly constructed instance is PENDING (the constructor does new AtomicReference<>(WorkflowStatus.PENDING)), so internalCancel() takes the else branch, cancel() returns false, publishStatusChange is never called, cleanUp never runs, and the entry remains. WorkflowDefinition.removeInstance is package-private, so there is no alternative.

The result is that cancel() on a PENDING instance looks like cleanup and silently does nothing — arguably the worse half of this report.

Why we hit it: our application needs the instance's id before it starts the run, because the id is written to a database row that claims ownership of a piece of work (so that a concurrently-arriving event can find the run). If that claim loses an optimistic-locking race, the run must not start — and the instance we created is then unreachable but permanently registered. We also hit it on the plainer path where start() itself throws.

What you expected to happen:

One of:

cancel() works on a PENDING instance — treat PENDING as cancellable, so it transitions to CANCELLED, publishes the status change and runs cleanUp. This seems the most consistent with the existing model: PENDING is a non-terminal state, and "cancel something that has not started" is the least surprising reading of cancel().
Or a public way to discard an unstarted instance (WorkflowDefinition.removeInstance(WorkflowInstance) made public, or WorkflowInstance.discard()).
Or registration deferred until start(), so instance(...) has no side effect on the definition. This is the cleanest but the most behaviour-changing, since activeInstance(id) would stop finding a created-not-started instance.

Happy to open a PR for whichever option you prefer — (1) looks like a small change in internalCancel/isActive.

How to reproduce it:

var definition = ...;
var instance = definition.instance(input);   // never started

assertThat(instance.cancel()).isFalse();                         // no-op
assertThat(definition.activeInstance(instance.id())).isPresent(); // still registered, forever

Environment:

io.serverlessworkflow:serverlessworkflow-impl-core 7.34.0.Final (as brought in by io.quarkiverse.flow:quarkus-flow 1.3.0)
Java 25

Verify the email intake and wrap up the feature (task 14 of #216)

Verify the email intake and wrap up the feature (task 14 of #216) (#287)

Well I would say that the obvious workaround, without any PR, is to call start and then cancel.
Pending is an intermediate step, it is expected that all restored instances are started at startup. Or they should not be restored at all.

[Fix open-workflow-specification#1816] Remove cancelled pending insta…

[Fix open-workflow-specification#1816] Remove restored instance cance…

@fjtirado

Thanks @fjtirado. Unfortunately start-then-cancel doesn't help in our case, because start() runs the workflow before cancel() can be called.

WorkflowMutableInstance.startExecution → status(RUNNING) → publishEvent → TaskExecutorHelper.processTaskList → AbstractTaskExecutor.apply is a chain of thenCompose calls on already-completed futures. It runs on the caller's thread until a task hands work to executorService() (fork, wait, run script/shell, listen). By the time start() returns:

every task before that point has already executed (all of them, for a fully synchronous workflow), and
onWorkflowStarted / onTaskStarted / onTaskCompleted have been published, so persistence, CloudEvents and metrics listeners have recorded a run.

In our scenario the run must not start: the instance id is written to a DB row to claim a piece of work, and if that claim loses the optimistic-lock race, nothing may execute.

Also, this isn't the restore path. We aren't restoring anything. We call the public definition.instance(input) and later decide whether to start() it. If PENDING is only meant to be an intermediate state, then a public instance() shouldn't leave a permanent entry in activeInstances with no way to remove it.

I wrote a reproducer against current main (7176bab) with a workflow of two synchronous set tasks. The assertions describe the behavior we'd expect, and both fail today:

var instance = definition.instance(Map.of());
assertThat(instance.cancel()).isTrue();
// -> Expecting value to be true but was false
//    (PENDING isn't in TaskExecutorHelper.isActive, so cancel() is a no-op and cleanUp never runs)

var instance = definition.instance(Map.of());
instance.start();
instance.cancel();
assertThat(listener.events).isEmpty();
// -> Expecting empty but was:
//    ["workflowStarted", "taskStarted:first", "taskCompleted:first",
//     "taskStarted:second", "taskCompleted:second"]

Full reproducer: impl/test/src/test/java/org/openworkflow/sdk/impl/test/UnstartedInstanceTest.java

package org.openworkflow.sdk.impl.test;

import static org.assertj.core.api.Assertions.assertThat;

import java.util.List;
import java.util.Map;
import java.util.concurrent.CopyOnWriteArrayList;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.openworkflow.sdk.api.WorkflowFormat;
import org.openworkflow.sdk.api.WorkflowReader;
import org.openworkflow.sdk.impl.WorkflowApplication;
import org.openworkflow.sdk.impl.WorkflowDefinition;
import org.openworkflow.sdk.impl.WorkflowInstance;
import org.openworkflow.sdk.impl.WorkflowStatus;
import org.openworkflow.sdk.impl.lifecycle.TaskCompletedEvent;
import org.openworkflow.sdk.impl.lifecycle.TaskStartedEvent;
import org.openworkflow.sdk.impl.lifecycle.WorkflowExecutionListener;
import org.openworkflow.sdk.impl.lifecycle.WorkflowStartedEvent;

/**
 * Reproducer for #1816: an instance obtained from {@link WorkflowDefinition#instance(Object)} that
 * the caller decides not to run must be discardable without executing any of its tasks.
 */
class UnstartedInstanceTest {

  private static final String WORKFLOW =
      """
      document:
        dsl: '1.0.0'
        namespace: test
        name: unstarted-instance
        version: '0.1.0'
      do:
        - first:
            set:
              step: one
        - second:
            set:
              step: two
      """;

  private static WorkflowApplication appl;
  private static RecordingListener listener;
  private static WorkflowDefinition definition;

  @BeforeAll
  static void init() {
    listener = new RecordingListener();
    appl =
        WorkflowApplication.builder().disableLifeCycleCEPublishing().withListener(listener).build();
    definition =
        appl.workflowDefinition(
            WorkflowReader.readWorkflowFromString(WORKFLOW, WorkflowFormat.YAML));
  }

  @AfterAll
  static void cleanup() {
    appl.close();
  }

  @BeforeEach
  void reset() {
    listener.events.clear();
  }

  @Test
  void cancelOnPendingInstanceDiscardsIt() {
    WorkflowInstance instance = definition.instance(Map.of());
    assertThat(instance.status()).isEqualTo(WorkflowStatus.PENDING);

    assertThat(instance.cancel()).as("cancel() on a PENDING instance").isTrue();
    assertThat(instance.status()).isEqualTo(WorkflowStatus.CANCELLED);
    assertThat(definition.activeInstance(instance.id()))
        .as("instance still registered in activeInstances after cancel()")
        .isEmpty();
  }

  @Test
  void startThenCancelDoesNotRunTasks() {
    WorkflowInstance instance = definition.instance(Map.of());

    // Workaround suggested in #1816: start() and immediately cancel()
    instance.start();
    instance.cancel();

    assertThat(listener.events)
        .as("lifecycle events published by start() before cancel() could run")
        .isEmpty();
    assertThat(instance.status()).isEqualTo(WorkflowStatus.CANCELLED);
  }

  static class RecordingListener implements WorkflowExecutionListener {
    final List<String> events = new CopyOnWriteArrayList<>();

    @Override
    public void onWorkflowStarted(WorkflowStartedEvent ev) {
      events.add("workflowStarted");
    }

    @Override
    public void onTaskStarted(TaskStartedEvent ev) {
      events.add("taskStarted:" + ev.taskContext().taskName());
    }

    @Override
    public void onTaskCompleted(TaskCompletedEvent ev) {
      events.add("taskCompleted:" + ev.taskContext().taskName());
    }
  }
}

Run with: mvn -pl impl/test -am -Dtest=UnstartedInstanceTest -Dsurefire.failIfNoSpecifiedTests=false test

So I think we still need one of the three options in the issue description. Your "PENDING is an intermediate step" point fits option 3 best (register the instance in activeInstances only on start()), though option 1 is the smallest change. Which would you prefer? I'm happy to open the PR.

One more way into this that the caller doesn't control, so start-then-cancel can't help: reading a persisted
instance registers one.

AbstractPersistenceInstanceReader.find/scanAll map every persisted row through WorkflowPersistenceInstance.of,
which builds a new instance when none is active, and the WorkflowMutableInstance constructor registers it
(definition.addInstance(this)). So a lookup from a JVM that doesn't host the run (another replica, or after a restart
with restore off) leaves a PENDING instance in activeInstances for good. That has two consequences:

The lookup reports a status the run doesn't have. The instance is PENDING whatever was persisted, so
find(...).status() can't tell a waiting run from anything else. (With the JPA store the persisted status is null
for a running or waiting run anyway, so a reader couldn't do much better, but PENDING looks like an answer.)
It poisons activeInstance(id) in that JVM. Afterwards the run looks local, and cancel() on it returns false
(the PENDING no-op described above).

Reproduced with sdk-java 7.35.3.Final through quarkus-flow 1.3.0 and quarkus-flow-jpa (a real run driven to WAITING,
then removed from activeInstances to stand in for another JVM):

Step	Result
`reader.find(definition, id)`	`WorkflowPersistenceInstance`, `PENDING`
`definition.activeInstance(id)` afterwards	present: the same instance, `PENDING`
`cancel()` on it	`false`; the persisted rows stay

The same code is on main (org.openworkflow.sdk.impl.persistence.WorkflowPersistenceInstance.of).

#1820 gives a way to clean up after the fact, but it doesn't stop a read from having the side effect. And with it,
cancelling the instance a read left behind publishes the cancellation, which WorkflowPersistenceListener turns into
aborted → removeProcessInstance: it deletes the persisted run, which is rarely what someone who only looked it up
wants. That's an argument for option 3 (register on start()), or at least for find/scanAll not registering what
they build until it's started.

Correct the spike's auto-restore findings and link upstream (#284)

@edeandrea

@edeandrea

I'm thinking we should automatically start when restoring (so restored instance cannot be in pending status and it is ok to not cancel what has not been started. so #1820 is not needed).
The reason I did not do that, restore in started was because I thought it give more flexibility, but not really, it is leading to weird scenarios that the one you spotted.

Edited:
Hmmm, no, because it will lead to other issues (you can read the instance for a different reason and then you will be starting the instance in two different JVMS, what is precisely what we want to avoid)
What really needs to be done is that the instance is not added to the map of active instances till is started (which is kind of weird) and change any code that assume that.
Still pending instances will not be cancelled, which makes sense to me (once that the pending instances are not kept anywhere, that should not be an issue)

@fjtirado

Thanks @fjtirado, registering on start() works for me. It removes both problems: a created-but-never-started instance no longer leaks, and a find/scanAll lookup no longer leaves anything in activeInstances. Agreed that cancel() on a PENDING instance can then stay a no-op, since there's nothing left to clean up.

Since #1820 takes a different approach (making PENDING cancellable), I'm happy to go either way: open my own PR for this, or review/comment on #1820 and let @NopeCharon rework it. Whatever you prefer. Either way, a few things the change will need to cover:

WorkflowPersistenceInstance.of is the only place on main that relies on registration at construction (activeInstance(id).orElseGet(...)). It still works for a run that has started in this JVM, but it no longer deduplicates unstarted ones, so two lookups of the same id return two separate instances.
Registering in startExecution: it should use putIfAbsent (or equivalent) so that starting a second instance with an id that's already active fails instead of silently replacing the running one.
Cleanup: removeInstance should remove only if the entry is still this same instance (remove(id, instance), as [Fix #1816] Allow cancelling an instance that has never been started #1820 already does). Otherwise a stale instance's cleanup can unregister a different one with the same id.

One leftover from my previous comment that this doesn't address: an instance returned by find/scanAll still reports PENDING, whatever its persisted status. For a read-only lookup, it would be more useful for status() to return the persisted status where the store has one (PersistenceWorkflowInfo.status()), or for the javadoc to say that the status of an unstarted, read instance isn't meaningful. I'm fine with either, but PENDING currently looks like a real answer.

Lets go for register with start. There is not problem on creating two pending instances with the same id as far as only one is started (which should now be controlled in start rather than in of)
About the leftover, lets document that the status of a read instance is not meaningful (thats not exactly true, but it is safer)
Ill add the new listener I mentioned in my previous comment with a different, future issue.

@fjtirado

Sounds good, thanks @fjtirado. To recap: register in start() (and enforce there that only one instance per id is active), drop the dedup in WorkflowPersistenceInstance.of, leave cancel() on PENDING as a no-op, and document that the status of an instance obtained by reading from persistence isn't meaningful.

Who should take it? I'm happy to open a new PR, or @NopeCharon can rework #1820 along these lines and I'll review. Either is fine with me.

Please open a new PR following these lines and lets close #1820.

#1827 opened!

[Fix open-workflow-specification#1816] Address javadoc review comments

[Fix #1816] Register workflow instances in the definition when started (

[7.x] [Fix #1816] Register workflow instances in the definition when …

Make the email intake survive restarts (single replica) (#297)

Navigation Menu

Repository navigation

A created-but-never-started WorkflowInstance stays in activeInstances, and cancel() cannot remove it #1816

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

A created-but-never-started WorkflowInstance stays in activeInstances, and cancel() cannot remove it #1816

Description

Activity

fjtirado commented on Nov 5, 2026

edeandrea commented on Nov 9, 2026

edeandrea commented on Nov 10, 2026

fjtirado commented on Nov 10, 2026

edeandrea commented on Nov 10, 2026

fjtirado commented on Nov 10, 2026

edeandrea commented on Nov 10, 2026

fjtirado commented on Nov 10, 2026

edeandrea commented on Nov 10, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions