Skip to content
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.

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

Fix FlowBuilder.next().end() infinite loop #4475

Closed

Conversation

injae-kim
Copy link
Contributor

related issue #4432

Motivation:

builder.next(createCompleteStep("stepA"))
       .end()
       .start();
--
[main] INFO org.springframework.batch.core.job.SimpleStepHandler - Executing step: [stepA]
[main] INFO org.springframework.batch.core.job.SimpleStepHandler - Executing step: [stepA]
[main] INFO org.springframework.batch.core.job.SimpleStepHandler - Executing step: [stepA]
... "stepA" never finishes

Modification:

  • If there's no step registered yet on FlowBuilder.next().end(), handle same as FlowBuilder.start().end() to avoid infinite loop

Result:

Comment on lines -253 to -256
State next = createState(input);
addTransition("COMPLETED", next);
addTransition("*", failedState);
this.currentState = next;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

		if (this.currentState == null) {
			doStart(input); // 1. currentState: input
		}

		State next = createState(input);
        // 2. input -> COMPLETED -> input -> COMPLETED -> input ..
		addTransition("COMPLETED", next); 

when currentState == null, I think we should not addTransition("COMPLETED", next); cause it makes infinite loop 🤔

Comment on lines +272 to +275
} else {
State state = createState(input);
tos.put(currentState.getName(), currentState);
currentState = state;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

When currrent == null, previous doFrom logic seems createState(input) twice (inside of doStart() and L273) so I fix it~!

@injae-kim
Copy link
Contributor Author

injae-kim commented Nov 7, 2023

Gentle ping to @fmbenhassine , can you review this PR? I need to fix this for my service🙇 thanks for your help!

@fmbenhassine
Copy link
Contributor

LGTM 👍 Rebased and merged as 4a67b22. Thank you for your contribution!

@injae-kim
Copy link
Contributor Author

Thanks a lot for review&merge! I'll make another contribution soon 🙇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Starting a flow with Flow#next makes the first step execute twice
2 participants