gnodet-bot commented on code in PR #27931:
URL: https://github.com/apache/camel/pull/27931#discussion_r4290646170
##########
dsl/camel-jbang/camel-jbang-plugin-tui/src/main/java/org/apache/camel/dsl/jbang/core/commands/tui/AiCliCommandExecutor.java:
##########
@@ -110,7 +107,7 @@ private void execute(ActiveCommand command) {
int exitCode = 1;
boolean interrupted = false;
try {
- synchronized (CamelJBangMain.class) {
+ synchronized (CliRunner.LOCK) {
Review Comment:
⚠️ **Redundant outer lock — `CliRunner.run()` already acquires `LOCK`
internally**
`invoke()` now calls `CliRunner.run(argv)` (the zero-timeout overload),
which calls `execute(commandLine, args, output)` on the same thread — and that
method opens `synchronized (LOCK)` itself. Java's intrinsic monitors are
reentrant, so this compiles and runs correctly, but the outer `synchronized
(CliRunner.LOCK)` here does not protect anything that `CliRunner` doesn't
already protect: by the time this block exits, `CliRunner.run()` has already
acquired and released the lock for the entire command execution.
The net effect is two nested lock acquisitions (reentrant, harmless) but
zero added protection. Remove the outer `synchronized` to make the intent
clear: the locking contract belongs to `CliRunner`, not to its callers.
```suggestion
exitCode = invoker.execute(command.request.argv(), new
CapturePrinter(output));
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
