Skip to content

Conversation

@squakez
Copy link
Contributor

@squakez squakez commented Nov 19, 2025

Description

Target

  • I checked that the commit is targeting the correct branch (Camel 4 uses the main branch)

Tracking

  • If this is a large change, bug fix, or code improvement, I checked there is a JIRA issue filed for the change (usually before you start working on it).

Apache Camel coding standards and style

  • I checked that each commit in the pull request has a meaningful subject line and body.
  • I have run mvn clean install -DskipTests locally from root folder and I have committed all auto-generated changes.

@squakez squakez requested a review from oscerd November 19, 2025 11:24
@github-actions
Copy link
Contributor

🌟 Thank you for your contribution to the Apache Camel project! 🌟

🤖 CI automation will test this PR automatically.

🐫 Apache Camel Committers, please review the following items:

  • First-time contributors require MANUAL approval for the GitHub Actions to run

  • You can use the command /component-test (camel-)component-name1 (camel-)component-name2.. to request a test from the test bot.

  • You can label PRs using build-all, build-dependents, skip-tests and test-dependents to fine-tune the checks executed by this PR.

  • Build and test logs are available in the Summary page. Only Apache Camel committers have access to the summary.

  • ⚠️ Be careful when sharing logs. Review their contents before sharing them publicly.

}

}
} catch (org.apache.kafka.common.errors.InterruptException ie) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@oscerd this one seems to be the only reason why the thread is interrupted. Can you double check?

@davsclaus
Copy link
Contributor

those forever loops should also check if the camel consumer has isRunAllowed so they can nicely exit when camel is shutting down or if you stop the route etc

@squakez
Copy link
Contributor Author

squakez commented Nov 19, 2025

those forever loops should also check if the camel consumer has isRunAllowed so they can nicely exit when camel is shutting down or if you stop the route etc

do you suggest to use that condition in the while loop as well? do you have any example to share by any chance? thanks!

@davsclaus
Copy link
Contributor

This PR is fine.

About the endless while loop (true) then another PR can attempt to correct this. So when Camel is shutting down the consumer is being called in the doStop method. And there it should stop its resources and threads etc. So those forever loops seems to not be stopped gracefully and rely on some thread interrupt that is messy and ugly.

Try to do a

 git grep "while (isRunAllowed()"

Or combinations of that. There are some components that does this.

@squakez
Copy link
Contributor Author

squakez commented Nov 20, 2025

This PR is fine.

About the endless while loop (true) then another PR can attempt to correct this. So when Camel is shutting down the consumer is being called in the doStop method. And there it should stop its resources and threads etc. So those forever loops seems to not be stopped gracefully and rely on some thread interrupt that is messy and ugly.

Try to do a

 git grep "while (isRunAllowed()"

Or combinations of that. There are some components that does this.

Okey, thanks for the hint. I will try to have a look and include in this PR if I can, otherwise I'll create a Jira issue to keep track of such an enhancement.

@squakez squakez marked this pull request as draft November 20, 2025 08:03
@squakez squakez marked this pull request as ready for review November 21, 2025 07:53
@squakez squakez force-pushed the chore/fix_endless_loops branch from 6e4f75f to 2ad3f5f Compare November 21, 2025 07:56
@squakez
Copy link
Contributor Author

squakez commented Nov 21, 2025

@davsclaus @oscerd this should be good now. I've maintained the running flag in the pulsar, just in case the interrupted exception was not caught at a higher level. We probably could have removed it though.

@squakez squakez force-pushed the chore/fix_endless_loops branch from 2ad3f5f to 9d8e1ff Compare November 21, 2025 08:04
@davsclaus
Copy link
Contributor

LGTM

@oscerd
Copy link
Contributor

oscerd commented Nov 21, 2025

Thx. LGTM

@squakez squakez force-pushed the chore/fix_endless_loops branch from 9d8e1ff to 93d45cd Compare November 21, 2025 10:22
@davsclaus davsclaus force-pushed the chore/fix_endless_loops branch from 93d45cd to 72ec614 Compare November 21, 2025 11:18
@squakez squakez merged commit f9e8df7 into apache:main Nov 21, 2025
4 checks passed
@squakez squakez deleted the chore/fix_endless_loops branch November 21, 2025 14:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants