Skip to content

##4194 pass signal to the next subscribed listener #4195

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
martin-grofcik wants to merge 6 commits into
base: main
Choose a base branch
from
Open

##4194 pass signal to the next subscribed listener #4195

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
e459d50
##4194 pass signal to the next subscribed listener
martin-grofcik Mar 26, 2026
154e53f
##4194 test fix
martin-grofcik Mar 26, 2026
5f5856a
##4194 test clarification
martin-grofcik Mar 26, 2026
eb1e872
#4194 do not return suspended definitions for signal subscription
martin-grofcik Mar 28, 2026
82bd8d3
#4194 allow to continue in execution for which definition is suspended.
martin-grofcik Apr 7, 2026
a9e14b9
#4194 return non process subscribers
martin-grofcik Apr 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 2 additions & 5 deletions modules/flowable-engine/src/main/java/org/flowable/engine/impl/event/SignalEventHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,14 @@
import org.flowable.engine.impl.util.ProcessInstanceHelper;
import org.flowable.engine.repository.ProcessDefinition;
import org.flowable.eventsubscription.service.impl.persistence.entity.EventSubscriptionEntity;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

/**
* @author Daniel Meyer
* @author Joram Barrez
*/
public class SignalEventHandler extends AbstractEventHandler {

public static final String EVENT_HANDLER_TYPE = "signal";

@Override
Expand All @@ -51,10 +52,6 @@ public void handleEvent(EventSubscriptionEntity eventSubscription, Object payloa
org.flowable.bpmn.model.Process process = ProcessDefinitionUtil.getProcess(processDefinitionId);
ProcessDefinition processDefinition = ProcessDefinitionUtil.getProcessDefinition(processDefinitionId);

if (processDefinition.isSuspended()) {
throw new FlowableException("Could not handle signal: process definition with id: " + processDefinitionId + " is suspended for " + eventSubscription);
}
Comment on lines -54 to -56
Copy link
Copy Markdown
Contributor

@filiphr filiphr Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is still incorrect. If someone explicitly triggers a suspended definition we should fail.

Copy link
Copy Markdown
Contributor Author

@martin-grofcik martin-grofcik Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The situation here is the same as for executions (processes). If process instance is suspended and signal is triggered for the suspended process instance, SignalEventHandler does not check whether process instance is suspended or not.
because

      and (
       (EVT.EXECUTION_ID_ is null) 
       or 
       (EVT.EXECUTION_ID_ is not null AND EXC.SUSPENSION_STATE_ = 1)
      )

and that's why SignalEventHandler does not expect suspended process instance to be provided here.

The contract is the same for definitions. Suspended definitions are not provided to SignalEventHandler


// Start process instance via the flow element linked to the event
FlowElement flowElement = process.getFlowElement(eventSubscription.getActivityId(), true);
if (flowElement == null) {
Expand Down
40 changes: 32 additions & 8 deletions ...able-engine/src/test/java/org/flowable/engine/test/bpmn/event/signal/SignalEventTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,25 @@ public void testSignalUserTask() {
.isExactlyInstanceOf(FlowableException.class);
}

@Test
@Deployment(resources={"org/flowable/engine/test/bpmn/event/signal/SignalEventTest.testSignalStartEvent.bpmn20.xml"})
public void testDuplicatedSuspendedSignalStartEventFromProcess() {
repositoryService.suspendProcessDefinitionByKey("processWithSignalStart1");

// An example of behavior when there is no subscription to the signal.
runtimeService.signalEventReceived("nonExisting");

// Starting the process that fires the signal should start 2 process
// instances that are listening on that signal. The suspended one is not included.
runtimeService.startProcessInstanceByKey("processWithSignalThrow");

// Verify
assertThat(runtimeService.createProcessInstanceQuery().count()).isEqualTo(2);
assertThat(taskService.createTaskQuery().list()).extracting(Task::getName)
.containsExactlyInAnyOrder("Task in process B", "Task in process C");

}

@Test
public void testSignalStartEventFromProcess() {

Expand Down Expand Up @@ -742,8 +761,12 @@ public void testSignalCatchSuspendedDefinition() {

runtimeService.startProcessInstanceByKey("throwSignal");

assertThat(createEventSubscriptionQuery().count()).isZero();
assertThat(runtimeService.createProcessInstanceQuery().count()).isZero();
assertThat(createEventSubscriptionQuery().count())
.as("Process definition is suspended and subscription is kept untouched despite of received signal.")
.isEqualTo(1);
assertThat(runtimeService.createProcessInstanceQuery().count())
.as("Process definition is suspended and process instance is kept untouched despite of received signal.")
.isEqualTo(1);
Copy link
Copy Markdown
Contributor

@filiphr filiphr Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what I was afraid of. This change here is not correct. The definition is suspended, which means that it's respective signal start events are suspended. However, in this particular case the event subscription is for a catch event, and this is linked to an execution. The SQL

and (
  (EVT.EXECUTION_ID_ is null) 
         or 
  (EVT.EXECUTION_ID_ is not null AND EXC.SUSPENSION_STATE_ = 1)
)
and (
  (EVT.PROC_DEF_ID_ is null)
    or
  (EVT.PROC_DEF_ID_ is not null AND DEF.SUSPENSION_STATE_ = 1)
)

The reason is that the process definition is always set, even when then execution is set. This means that if we want to do something we need to change the SQL a bit. I think that we need to remove the join and do something like:

 select * 
    from ACT_RU_EVENT_SUBSCR EVT
    left outer join ACT_RU_EXECUTION EXC on EVT.EXECUTION_ID_ = EXC.ID_
    where EVENT_TYPE_ = 'signal'
      and EVENT_NAME_ = #{parameter.eventName, jdbcType=NVARCHAR}
      and (
       (EVT.EXECUTION_ID_ is null and EVT.PROC_DEF_ID_ is not null and exists (select 1 from ACT_RE_PROCDEF DEF where DEF.ID_ = EVT.PROC_DEF_ID_ and DEF.SUSPENSION_STATE_ = 1)) 
       or 
       (EVT.EXECUTION_ID_ is not null AND exists (select 1 from ACT_RU_EXECUTION EXC where EXC.ID_ = EVT.EXECUTION_ID_ and EXC.SUSPENSION_STATE_ = 1) 
      )

Copy link
Copy Markdown
Contributor Author

@martin-grofcik martin-grofcik Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added check for other scope types too.

Copy link
Copy Markdown
Contributor Author

@martin-grofcik martin-grofcik Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@filiphr any update?

}

@Test
Expand Down Expand Up @@ -805,12 +828,13 @@ public void testSignalStartEventWithSuspendedDefinition() {

repositoryService.suspendProcessDefinitionByKey("processWithSignalStart1");

assertThatThrownBy(() -> runtimeService.startProcessInstanceByKey("processWithSignalThrow"))
.as("Suspended process definition should fail")
.isExactlyInstanceOf(FlowableException.class);
runtimeService.startProcessInstanceByKey("processWithSignalThrow");

// Verify
assertThat(runtimeService.createProcessInstanceQuery().count()).isZero();
assertThat(runtimeService.createProcessInstanceQuery().list())
.as("Signal throw event is swallowed for suspended process")
.extracting(ProcessInstance::getProcessDefinitionKey)
.containsExactlyInAnyOrder("processWithSignalStart2","processWithSignalStart3");

repositoryService.activateProcessDefinitionByKey("processWithSignalStart1");

Expand All @@ -819,8 +843,8 @@ public void testSignalStartEventWithSuspendedDefinition() {
runtimeService.startProcessInstanceByKey("processWithSignalThrow");

// Verify
assertThat(runtimeService.createProcessInstanceQuery().count()).isEqualTo(3);
assertThat(taskService.createTaskQuery().count()).isEqualTo(3);
assertThat(runtimeService.createProcessInstanceQuery().count()).isEqualTo(5);
assertThat(taskService.createTaskQuery().count()).isEqualTo(5);

// Cleanup
cleanup();
Expand Down
8 changes: 7 additions & 1 deletion .../resources/org/flowable/eventsubscription/service/db/mapping/entity/EventSubscription.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,12 +240,18 @@
select *
from ${prefix}ACT_RU_EVENT_SUBSCR EVT
left outer join ${prefix}ACT_RU_EXECUTION EXC on EVT.EXECUTION_ID_ = EXC.ID_
left outer join ${prefix}ACT_RE_PROCDEF DEF on EVT.PROC_DEF_ID_ = DEF.ID_
where EVENT_TYPE_ = 'signal'
and EVENT_NAME_ = #{parameter.eventName, jdbcType=NVARCHAR}
and (
(EVT.EXECUTION_ID_ is null)
or
(EVT.EXECUTION_ID_ is not null AND EXC.SUSPENSION_STATE_ = 1)
(EVT.EXECUTION_ID_ is not null AND EXC.SUSPENSION_STATE_ = 1)
)
and (
(EVT.PROC_DEF_ID_ is null)
or
(EVT.PROC_DEF_ID_ is not null AND DEF.SUSPENSION_STATE_ = 1)
)
<if test="parameter.tenantId != null">
and EVT.TENANT_ID_ = #{parameter.tenantId, jdbcType=NVARCHAR}
Expand Down