You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by "gnodet (via GitHub)" <gi...@apache.org> on 2026/07/30 08:34:21 UTC

[PR] CAMEL-20199: Replace synchronized with ReentrantLock in AI components [camel]

gnodet opened a new pull request, #25245:
URL: https://github.com/apache/camel/pull/25245

   ## Summary
   
   - Replace `synchronized` blocks with `ReentrantLock` in three files within the `camel-ai` parent module to improve virtual thread compatibility (part of CAMEL-20199)
   - `GuardedTaskStore`: Replace internal `Object lifecycleLock` with `ReentrantLock`, expose via `getLock()` so external callers can coordinate
   - `A2AProgress`: Use `GuardedTaskStore.getLock()` to share the same lock instance, with a `ConcurrentHashMap`-based fallback for non-guarded stores
   - `AbstractTaskPredictor`: Replace `synchronized (this)` with a private `ReentrantLock` field
   
   ## Test plan
   
   - [x] `mvn test -B` in `components/camel-ai/camel-a2a` -- 506 tests pass, 0 failures
   - [x] `mvn test -B` in `components/camel-ai/camel-huggingface` -- BUILD SUCCESS
   - [x] `mvn formatter:format impsort:sort` passes without reverting changes
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   
   Co-Authored-By: Claude Opus 4.6 <no...@anthropic.com>


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] CAMEL-20199: Replace synchronized with ReentrantLock in AI components [camel]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #25245:
URL: https://github.com/apache/camel/pull/25245#issuecomment-5128714188

   :star2: Thank you for your contribution to the Apache Camel project! :star2:
     :robot: CI automation will test this PR automatically.
   
     :camel: 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 although they are normally detected and executed by CI.
     * You can label PRs using `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](https://camel.apache.org/community/team/#committers) have access to the summary.
     
     :warning: Be careful when sharing logs. Review their contents before sharing them publicly.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] CAMEL-20199: Replace synchronized with ReentrantLock in AI components [camel]

Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet merged PR #25245:
URL: https://github.com/apache/camel/pull/25245


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] CAMEL-20199: Replace synchronized with ReentrantLock in AI components [camel]

Posted by "apupier (via GitHub)" <gi...@apache.org>.
apupier commented on code in PR #25245:
URL: https://github.com/apache/camel/pull/25245#discussion_r3681896818


##########
components/camel-ai/camel-a2a/src/main/java/org/apache/camel/component/a2a/A2AProgress.java:
##########
@@ -30,16 +32,31 @@
 import org.apache.camel.component.a2a.model.TaskStatus;
 import org.apache.camel.component.a2a.model.TextPart;
 import org.apache.camel.component.a2a.state.A2ATaskStore;
+import org.apache.camel.component.a2a.state.GuardedTaskStore;
 
 /**
  * Static utility for emitting A2A progress updates from any exchange. Resolves the task store via the exchange's
  * {@code fromEndpoint} (primary) or by scanning the A2A component's endpoints (fallback).
  */
 public final class A2AProgress {
 
+    private static final ConcurrentHashMap<A2ATaskStore, ReentrantLock> STORE_LOCKS = new ConcurrentHashMap<>();
+

Review Comment:
   Unless I miss it but this map is never cleaned. When a store is removed, I would expect that the corresponding reentrant lock is removed



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] CAMEL-20199: Replace synchronized with ReentrantLock in AI components [camel]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #25245:
URL: https://github.com/apache/camel/pull/25245#issuecomment-5129130921

   <!-- ci-tested-modules -->
   :test_tube: **CI tested the following changed modules:**
   
   - `components/camel-ai/camel-a2a`
   - `components/camel-ai/camel-huggingface`
   
   ---
   
   <details><summary>:microscope: Scalpel shadow comparison — Scalpel: 10 tested, 29 compile-only — current: 10 all tested</summary>
   
   [Maveniverse Scalpel](https://github.com/maveniverse/scalpel) detected **39 affected modules** (current approach: 10).
   
   <details><summary>:warning: Modules only in Scalpel (29)</summary>
   
   - `apache-camel`
   - `camel-allcomponents`
   - `camel-catalog`
   - `camel-catalog-console`
   - `camel-catalog-lucene`
   - `camel-catalog-maven`
   - `camel-catalog-suggest`
   - `camel-componentdsl`
   - `camel-csimple-maven-plugin`
   - `camel-endpointdsl`
   - `camel-endpointdsl-support`
   - `camel-itest`
   - `camel-jbang-core`
   - `camel-jbang-it`
   - `camel-jbang-main`
   - `camel-jbang-plugin-edit`
   - `camel-jbang-plugin-generate`
   - `camel-jbang-plugin-kubernetes`
   - `camel-jbang-plugin-test`
   - `camel-kamelet-main`
   - `camel-launcher`
   - `camel-report-maven-plugin`
   - `camel-route-parser`
   - `camel-yaml-dsl`
   - `camel-yaml-dsl-deserializers`
   - `camel-yaml-dsl-maven-plugin`
   - `coverage`
   - `docs`
   - `dummy-component`
   
   </details>
   
   **Skip-tests mode would test 10 modules** (2 direct + 8 downstream), **skip tests for 29** (generated code, meta-modules)
   
   <details><summary>Modules Scalpel would test (10)</summary>
   
   - `camel-a2a`
   - `camel-huggingface`
   - `camel-jbang-mcp`
   - `camel-jbang-plugin-mcp`
   - `camel-jbang-plugin-route-parser`
   - `camel-jbang-plugin-tui`
   - `camel-jbang-plugin-validate`
   - `camel-launcher-container`
   - `camel-yaml-dsl-validator`
   - `camel-yaml-dsl-validator-maven-plugin`
   
   </details>
   
   <details><summary>Modules with tests skipped (29)</summary>
   
   - `apache-camel`
   - `camel-allcomponents`
   - `camel-catalog`
   - `camel-catalog-console`
   - `camel-catalog-lucene`
   - `camel-catalog-maven`
   - `camel-catalog-suggest`
   - `camel-componentdsl`
   - `camel-csimple-maven-plugin`
   - `camel-endpointdsl`
   - `camel-endpointdsl-support`
   - `camel-itest`
   - `camel-jbang-core`
   - `camel-jbang-it`
   - `camel-jbang-main`
   - `camel-jbang-plugin-edit`
   - `camel-jbang-plugin-generate`
   - `camel-jbang-plugin-kubernetes`
   - `camel-jbang-plugin-test`
   - `camel-kamelet-main`
   - `camel-launcher`
   - `camel-report-maven-plugin`
   - `camel-route-parser`
   - `camel-yaml-dsl`
   - `camel-yaml-dsl-deserializers`
   - `camel-yaml-dsl-maven-plugin`
   - `coverage`
   - `docs`
   - `dummy-component`
   
   </details>
   
   > :information_source: Shadow mode — Scalpel observes but does not affect test execution. [Learn more](https://github.com/maveniverse/scalpel)
   
   </details>
   
   <details><summary>All tested modules (39 modules)</summary>
   
   - Camel :: AI :: A2A
   - Camel :: AI :: Hugging Face
   - Camel :: All Components Sync point
   - Camel :: Assembly
   - Camel :: Catalog :: CSimple Maven Plugin (deprecated)
   - Camel :: Catalog :: Camel Catalog
   - Camel :: Catalog :: Camel Report Maven Plugin
   - Camel :: Catalog :: Camel Route Parser
   - Camel :: Catalog :: Console
   - Camel :: Catalog :: Dummy Component
   - Camel :: Catalog :: Lucene (deprecated)
   - Camel :: Catalog :: Maven
   - Camel :: Catalog :: Suggest
   - Camel :: Component DSL
   - Camel :: Coverage
   - Camel :: Docs
   - Camel :: Endpoint DSL
   - Camel :: Endpoint DSL :: Support
   - Camel :: Integration Tests
   - Camel :: JBang :: Core
   - Camel :: JBang :: Integration tests
   - Camel :: JBang :: MCP
   - Camel :: JBang :: Main
   - Camel :: JBang :: Plugin :: Edit
   - Camel :: JBang :: Plugin :: Generate
   - Camel :: JBang :: Plugin :: Kubernetes
   - Camel :: JBang :: Plugin :: MCP
   - Camel :: JBang :: Plugin :: Route Parser
   - Camel :: JBang :: Plugin :: TUI
   - Camel :: JBang :: Plugin :: Testing
   - Camel :: JBang :: Plugin :: Validate
   - Camel :: Kamelet Main
   - Camel :: Launcher
   - Camel :: Launcher :: Container
   - Camel :: YAML DSL
   - Camel :: YAML DSL :: Deserializers
   - Camel :: YAML DSL :: Maven Plugins
   - Camel :: YAML DSL :: Validator
   - Camel :: YAML DSL :: Validator Maven Plugin
   
   </details>
   
   ---
   :gear: [View full build and test results](https://github.com/apache/camel/actions/runs/30527222956)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] CAMEL-20199: Replace synchronized with ReentrantLock in AI components [camel]

Posted by "apupier (via GitHub)" <gi...@apache.org>.
apupier commented on code in PR #25245:
URL: https://github.com/apache/camel/pull/25245#discussion_r3682490912


##########
components/camel-ai/camel-a2a/src/main/java/org/apache/camel/component/a2a/A2AEndpoint.java:
##########


Review Comment:
   shouldn't it be better to clean it when the TaskStore service is stopped?
   
   Here I fear that the lock could be removed for the taskstore which are not owned by this A2AEndpoint



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] CAMEL-20199: Replace synchronized with ReentrantLock in AI components [camel]

Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #25245:
URL: https://github.com/apache/camel/pull/25245#discussion_r3682387803


##########
components/camel-ai/camel-a2a/src/main/java/org/apache/camel/component/a2a/A2AProgress.java:
##########
@@ -30,16 +32,31 @@
 import org.apache.camel.component.a2a.model.TaskStatus;
 import org.apache.camel.component.a2a.model.TextPart;
 import org.apache.camel.component.a2a.state.A2ATaskStore;
+import org.apache.camel.component.a2a.state.GuardedTaskStore;
 
 /**
  * Static utility for emitting A2A progress updates from any exchange. Resolves the task store via the exchange's
  * {@code fromEndpoint} (primary) or by scanning the A2A component's endpoints (fallback).
  */
 public final class A2AProgress {
 
+    private static final ConcurrentHashMap<A2ATaskStore, ReentrantLock> STORE_LOCKS = new ConcurrentHashMap<>();
+

Review Comment:
   Good catch, thank you! Added a package-private `removeStoreLock(A2ATaskStore)` method to `A2AProgress` and wired it into `A2AEndpoint.cleanupEndpointResources()` so the lock entry is removed when the endpoint shuts down. This prevents the static `STORE_LOCKS` map from leaking entries.
   
   _Claude Code on behalf of gnodet_



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

Re: [PR] CAMEL-20199: Replace synchronized with ReentrantLock in AI components [camel]

Posted by "gnodet (via GitHub)" <gi...@apache.org>.
gnodet commented on code in PR #25245:
URL: https://github.com/apache/camel/pull/25245#discussion_r3682566620


##########
components/camel-ai/camel-a2a/src/main/java/org/apache/camel/component/a2a/A2AEndpoint.java:
##########


Review Comment:
   Good point — moved the `removeStoreLock()` call inside the `if (taskStoreOwned)` guard so it only cleans up locks for stores exclusively owned by this endpoint. Non-owned (registry-discovered) stores are always wrapped in `GuardedTaskStore`, which provides its own lock via `getLock()` and never enters the static `STORE_LOCKS` map, so they were never at risk of premature removal — but the guard makes the intent explicit and prevents any future surprise.
   
   _Claude Code on behalf of 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@camel.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org