You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by "Croway (via GitHub)" <gi...@apache.org> on 2026/09/11 08:43:38 UTC
[PR] CAMEL-24690: camel-python - evaluate under the interpreter lock, the globals are shared [camel]
Croway opened a new pull request, #26311:
URL: https://github.com/apache/camel/pull/26311
`PythonExpression` keeps one `PythonInterpreter` per expression and `evaluate()` sets `exchange`, `body`, `headers` and `properties` into its globals, runs the compiled code and calls `cleanup()`, with no synchronization. Concurrent exchanges interleave those steps: one exchange can be evaluated with another one's body, and `cleanup()` runs while another thread is mid-eval. `PythonLanguage.evaluate` has the same race on its shared interpreter.
Binding, running and cleaning up now happen under the interpreter's monitor in both places. A new test evaluates one expression from 8 threads, 200 rounds each, and checks every result. Evaluations of one expression are serialized, which is the smallest correct fix for this legacy component; per-call locals would be the faster follow-up.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
_Claude Code on behalf of Croway_
--
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-24690: camel-python - evaluate under the interpreter lock, the globals are shared [camel]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #26311:
URL: https://github.com/apache/camel/pull/26311#issuecomment-5632227619
<!-- ci-tested-modules -->
:test_tube: **CI tested the following changed modules:**
- `components/camel-python`
---
<details><summary>:microscope: Scalpel shadow comparison — Scalpel: 9 tested, 27 compile-only — current: 9 all tested</summary>
[Maveniverse Scalpel](https://github.com/maveniverse/scalpel) detected **36 affected modules** (current approach: 9).
<details><summary>:warning: Modules only in Scalpel (27)</summary>
- `apache-camel`
- `camel-allcomponents`
- `camel-catalog`
- `camel-catalog-console`
- `camel-catalog-maven`
- `camel-catalog-suggest`
- `camel-componentdsl`
- `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 9 modules** (1 direct + 8 downstream), **skip tests for 27** (generated code, meta-modules)
<details><summary>Modules Scalpel would test (9)</summary>
- `camel-jbang-mcp`
- `camel-jbang-plugin-mcp`
- `camel-jbang-plugin-route-parser`
- `camel-jbang-plugin-tui`
- `camel-jbang-plugin-validate`
- `camel-launcher-container`
- `camel-python`
- `camel-yaml-dsl-validator`
- `camel-yaml-dsl-validator-maven-plugin`
</details>
<details><summary>Modules with tests skipped (27)</summary>
- `apache-camel`
- `camel-allcomponents`
- `camel-catalog`
- `camel-catalog-console`
- `camel-catalog-maven`
- `camel-catalog-suggest`
- `camel-componentdsl`
- `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 (36 modules, 4m 34s total)</summary>
**Total reactor time:** 4m 34s
| Module | Duration | Status |
| --- | --- | --- |
| Camel :: Launcher | 49.7s | SUCCESS |
| Camel :: JBang :: MCP | 37.8s | SUCCESS |
| Camel :: JBang :: Plugin :: TUI | 31.4s | SUCCESS |
| Camel :: Component DSL | 19.7s | SUCCESS |
| Camel :: Catalog :: Camel Catalog | 18.6s | SUCCESS |
| Camel :: JBang :: Plugin :: Kubernetes | 15.7s | SUCCESS |
| Camel :: Docs | 14.5s | SUCCESS |
| Camel :: Python | 10.0s | SUCCESS |
| Camel :: Catalog :: Camel Route Parser | 8.4s | SUCCESS |
| Camel :: JBang :: Plugin :: Testing | 8.2s | SUCCESS |
| Camel :: Kamelet Main | 8.0s | SUCCESS |
| Camel :: Catalog :: Camel Report Maven Plugin | 7.7s | SUCCESS |
| Camel :: YAML DSL :: Deserializers | 6.7s | SUCCESS |
| Camel :: All Components Sync point | 5.3s | SUCCESS |
| Camel :: YAML DSL :: Validator | 5.0s | SUCCESS |
| Camel :: YAML DSL :: Maven Plugins | 3.6s | SUCCESS |
| Camel :: Catalog :: Maven | 3.4s | SUCCESS |
| Camel :: YAML DSL :: Validator Maven Plugin | 2.6s | SUCCESS |
| Camel :: Catalog :: Suggest (deprecated) | 2.4s | SUCCESS |
| Camel :: Catalog :: Dummy Component | 2.3s | SUCCESS |
| Camel :: Assembly | 1.7s | SUCCESS |
| Camel :: Coverage | 1.4s | SUCCESS |
| Camel :: JBang :: Plugin :: Edit | 1.4s | SUCCESS |
| Camel :: JBang :: Integration tests | 1.3s | SUCCESS |
| Camel :: Endpoint DSL :: Support | 1.2s | SUCCESS |
| Camel :: JBang :: Plugin :: Generate | 1.0s | SUCCESS |
| Camel :: Catalog :: Console | 1.0s | SUCCESS |
| Camel :: JBang :: Plugin :: Validate | 0.9s | SUCCESS |
| Camel :: Launcher :: Container | 0.7s | SUCCESS |
| Camel :: JBang :: Main | 0.6s | SUCCESS |
| Camel :: JBang :: Plugin :: MCP | 0.5s | SUCCESS |
| Camel :: JBang :: Plugin :: Route Parser | 0.5s | SUCCESS |
| Camel :: Endpoint DSL | n/a | |
| Camel :: Integration Tests | n/a | |
| Camel :: JBang :: Core | n/a | |
| Camel :: YAML DSL | n/a | |
**Top 20 slowest modules:**
- `Camel :: Launcher` (49.7s)
- `Camel :: JBang :: MCP` (37.8s)
- `Camel :: JBang :: Plugin :: TUI` (31.4s)
- `Camel :: Component DSL` (19.7s)
- `Camel :: Catalog :: Camel Catalog` (18.6s)
- `Camel :: JBang :: Plugin :: Kubernetes` (15.7s)
- `Camel :: Docs` (14.5s)
- `Camel :: Python` (10.0s)
- `Camel :: Catalog :: Camel Route Parser` (8.4s)
- `Camel :: JBang :: Plugin :: Testing` (8.2s)
- `Camel :: Kamelet Main` (8.0s)
- `Camel :: Catalog :: Camel Report Maven Plugin` (7.7s)
- `Camel :: YAML DSL :: Deserializers` (6.7s)
- `Camel :: All Components Sync point` (5.3s)
- `Camel :: YAML DSL :: Validator` (5.0s)
- `Camel :: YAML DSL :: Maven Plugins` (3.6s)
- `Camel :: Catalog :: Maven` (3.4s)
- `Camel :: YAML DSL :: Validator Maven Plugin` (2.6s)
- `Camel :: Catalog :: Suggest (deprecated)` (2.4s)
- `Camel :: Catalog :: Dummy Component` (2.3s)
</details>
---
:gear: [View full build and test results](https://github.com/apache/camel/actions/runs/34580592582)
--
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-24690: camel-python - evaluate under the interpreter lock, the globals are shared [camel]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #26311:
URL: https://github.com/apache/camel/pull/26311#issuecomment-5631827924
: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-24690: camel-python - evaluate under the interpreter lock, the globals are shared [camel]
Posted by "Croway (via GitHub)" <gi...@apache.org>.
Croway commented on PR #26311:
URL: https://github.com/apache/camel/pull/26311#issuecomment-5636360281
Thanks. In 596fde80b580 the compile path is inside the lock and the monitor is a private object in both classes.
On the per-thread interpreter: I kept the lock deliberately. `threadLocalStateInterpreter` would remove the serialization, but it changes what a script sees across messages on the same thread (globals written by one evaluation stay visible to the next on that thread, where `cleanup()` currently resets them), and camel-python is the legacy Jython binding while camel-python3 is where throughput work belongs. The PR description now records that trade-off; if throughput on this component matters to someone, the thread-local interpreter is the next step and the new test is the guard for it.
Also added to the JIRA, as you suggested, that the test fails hard on main: every thread was reading another exchange's body and headers, so this is a correctness bug, not a tidy-up.
_Claude Code on behalf of Croway_
--
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-24690: camel-python - evaluate under the interpreter lock, the globals are shared [camel]
Posted by "Croway (via GitHub)" <gi...@apache.org>.
Croway merged PR #26311:
URL: https://github.com/apache/camel/pull/26311
--
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-24690: camel-python - evaluate under the interpreter lock, the globals are shared [camel]
Posted by "davsclaus (via GitHub)" <gi...@apache.org>.
davsclaus commented on code in PR #26311:
URL: https://github.com/apache/camel/pull/26311#discussion_r3989772222
##########
components/camel-python/src/main/java/org/apache/camel/language/python/PythonLanguage.java:
##########
@@ -75,19 +75,22 @@ public <T> T evaluate(String script, Map<String, Object> bindings, Class<T> resu
}
}
- try {
- if (bindings != null) {
- bindings.forEach(compiler::set);
- }
- PyObject out = compiler.eval(code);
- if (out != null) {
- String value = out.toString();
- return getCamelContext().getTypeConverter().convertTo(resultType, value);
+ // the interpreter is shared by every caller of this method: bind, run and clean up under one lock
+ synchronized (compiler) {
Review Comment:
**The compile a few lines above this is on the same shared interpreter, but outside the lock.**
`PythonInterpreter.compile(Reader, String)` is not a pure function of its argument — it reads `this.cflags`, passes that same mutable `CompilerFlags` instance into `ParserFacade.parseExpressionOrModule(...)` (which writes back any `from __future__` flags it finds), and then calls `setSystemState()`, which installs this interpreter's `PySystemState` as the *thread's* current one.
So two threads missing the cache on different scripts mutate one `CompilerFlags`, and a compiling thread races the `setSystemState()`/`eval` of a thread already inside this `synchronized` block. Same shared-mutable-interpreter race the PR fixes on the evaluation path, just on the compile path.
Pulling the cache miss inside the lock closes it and also makes the double-checked lookup honest:
```java
synchronized (compiler) {
PyCode code = getCompiledScriptFromCache(script);
if (code == null) {
try {
code = compiler.compile(script);
addCompiledScriptToCache(script, code);
} catch (Exception e) {
throw new ExpressionIllegalSyntaxException(script, e);
}
}
try {
...
```
(`PythonExpression`'s constructor compiles against its own private interpreter before the expression is published, so that one is fine as-is.)
--
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-24690: camel-python - evaluate under the interpreter lock, the globals are shared [camel]
Posted by "Croway (via GitHub)" <gi...@apache.org>.
Croway commented on code in PR #26311:
URL: https://github.com/apache/camel/pull/26311#discussion_r3990417784
##########
components/camel-python/src/main/java/org/apache/camel/language/python/PythonLanguage.java:
##########
@@ -75,19 +75,22 @@ public <T> T evaluate(String script, Map<String, Object> bindings, Class<T> resu
}
}
- try {
- if (bindings != null) {
- bindings.forEach(compiler::set);
- }
- PyObject out = compiler.eval(code);
- if (out != null) {
- String value = out.toString();
- return getCamelContext().getTypeConverter().convertTo(resultType, value);
+ // the interpreter is shared by every caller of this method: bind, run and clean up under one lock
+ synchronized (compiler) {
Review Comment:
Applied in 596fde80b580: the cache miss and compile now run inside the same lock as the bind/eval/cleanup, and both classes lock a private Object instead of the interpreter.
_Claude Code on behalf of Croway_
--
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