Use exec to ensure JVM receives SIGTERM for graceful shutdown#4600
Use exec to ensure JVM receives SIGTERM for graceful shutdown#4600JeethJJ wants to merge 1 commit into
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe startup script now initializes ChangesJava Process Startup Optimization
🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
distribution/kernel/carbon-home/bin/wso2server.sh (1)
300-345:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUnconditional
execbreaks the script’s restart loop semantics.At Line 302,
execreplaces the shell, so Line 344 (status=$?) never runs and thewhileloop cannot handleSTART_EXIT_STATUSanymore. This can regress script-driven restart flow (seeSystemRestarter.restart()usingSystem.exit(EXIT_CODE)forwso2server.shlaunches).Suggested adjustment (use
execonly when this script is PID 1)while [ "$status" = "$START_EXIT_STATUS" ] do - exec $JAVACMD \ + if [ "$$" -eq 1 ]; then + exec "$JAVACMD" \ + else + "$JAVACMD" \ + fi \ -Xbootclasspath/a:"$CARBON_XBOOTCLASSPATH" \ $JVM_MEM_OPTS \ @@ -Dcarbon.new.config.dir.path="$CARBON_HOME/repository/resources/conf" \ org.wso2.carbon.bootstrap.Bootstrap $* status=$? done🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@distribution/kernel/carbon-home/bin/wso2server.sh` around lines 300 - 345, The loop uses an unconditional exec which replaces the shell so the exit code assignment (status=$?) and restart logic never run; modify the block in wso2server.sh around the while [ "$status" = "$START_EXIT_STATUS" ] loop to either (A) remove exec and invoke the JVM command normally so the script retains control and status=$? executes, or (B) guard the exec so it only runs when the script is PID 1 (check $$ -eq 1) and otherwise run the JVM without exec; update the invocation of org.wso2.carbon.bootstrap.Bootstrap $* accordingly so START_EXIT_STATUS logic and SystemRestarter-driven restarts work.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@distribution/kernel/carbon-home/bin/wso2server.sh`:
- Around line 300-345: The loop uses an unconditional exec which replaces the
shell so the exit code assignment (status=$?) and restart logic never run;
modify the block in wso2server.sh around the while [ "$status" =
"$START_EXIT_STATUS" ] loop to either (A) remove exec and invoke the JVM command
normally so the script retains control and status=$? executes, or (B) guard the
exec so it only runs when the script is PID 1 (check $$ -eq 1) and otherwise run
the JVM without exec; update the invocation of
org.wso2.carbon.bootstrap.Bootstrap $* accordingly so START_EXIT_STATUS logic
and SystemRestarter-driven restarts work.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 69bede93-297f-413e-af64-a3fecd98fbea
📒 Files selected for processing (1)
distribution/kernel/carbon-home/bin/wso2server.sh
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@distribution/kernel/carbon-home/bin/wso2server.sh`:
- Around line 299-304: The restart loop fails because JAVA_LAUNCH_PREFIX is set
to "exec", causing the JVM launch line to replace the shell and preventing the
while [ "$status" = "$START_EXIT_STATUS" ] loop from observing $?. Fix by only
using exec for one-shot launches and keep the restart-loop path non-exec: detect
whether the script will enter the START_EXIT_STATUS restart loop (the while [
"$status" = "$START_EXIT_STATUS" ] block) and ensure JAVA_LAUNCH_PREFIX is empty
for that case; reserve JAVA_LAUNCH_PREFIX="exec" only for the PID1/one-shot code
path (or refactor to a separate variable/mode for exec vs wrapper-loop) so
status=$? after the JVM exit is always captured.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: 9bdf721e-b409-4726-83a8-b1c3af81d56a
📒 Files selected for processing (1)
distribution/kernel/carbon-home/bin/wso2server.sh
| JAVA_LAUNCH_PREFIX="" | ||
| if [ $$ -eq 1 ]; then | ||
| if [ -f "/.dockerenv" ] || [ -f "/run/.containerenv" ] || grep -qE '/(docker|kubepods|containerd|crio)/' /proc/1/cgroup 2>/dev/null; then | ||
| JAVA_LAUNCH_PREFIX="exec" | ||
| fi | ||
| fi |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify that `exec` is used in the same loop that depends on `$?` for restart.
set -euo pipefail
FILE="distribution/kernel/carbon-home/bin/wso2server.sh"
echo "== Restart loop contract =="
sed -n '278,352p' "$FILE"
echo
echo "== Key markers =="
rg -n 'START_EXIT_STATUS|while \[ "\$status" = "\$START_EXIT_STATUS" \]|JAVA_LAUNCH_PREFIX|^\s*\$JAVA_LAUNCH_PREFIX \$JAVACMD|status=\$\?' "$FILE"Repository: wso2/carbon-kernel
Length of output: 3783
Fix PID1 launch: don’t exec inside the START_EXIT_STATUS=121 restart loop
In distribution/kernel/carbon-home/bin/wso2server.sh, JAVA_LAUNCH_PREFIX="exec" (299-304) causes the JVM invocation inside while [ "$status" = "$START_EXIT_STATUS" ] (~307-351) to replace the shell at the launch line (~309). As a result, status=$? (~351) and the loop re-entry never execute, so the script can’t restart based on exit code 121.
Keep the restart-loop path non-exec (so $? is captured), and only use exec in a separate one-shot mode where restarting is not required (or refactor signal-forwarding without replacing the wrapper loop).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@distribution/kernel/carbon-home/bin/wso2server.sh` around lines 299 - 304,
The restart loop fails because JAVA_LAUNCH_PREFIX is set to "exec", causing the
JVM launch line to replace the shell and preventing the while [ "$status" =
"$START_EXIT_STATUS" ] loop from observing $?. Fix by only using exec for
one-shot launches and keep the restart-loop path non-exec: detect whether the
script will enter the START_EXIT_STATUS restart loop (the while [ "$status" =
"$START_EXIT_STATUS" ] block) and ensure JAVA_LAUNCH_PREFIX is empty for that
case; reserve JAVA_LAUNCH_PREFIX="exec" only for the PID1/one-shot code path (or
refactor to a separate variable/mode for exec vs wrapper-loop) so status=$?
after the JVM exit is always captured.
2 participants
Problem
When deploying WSO2 Identity Server in a Docker container, the JVM does not receive SIGTERM on container stop, resulting in a forceful kill after the grace period with no graceful shutdown.
The root cause is in
wso2server.sh— the Java process is launched as a child of the shell:This makes the shell PID 1 inside the container, and since the shell does not forward signals to its children, SIGTERM sent by Docker is never received by the JVM.
Fix
Add
execbefore$JAVACMDofwso2server.sh:execreplaces the shell process with the JVM, making Java PID 1 directly. SIGTERM is now received by the JVM and graceful shutdown is triggered.Verification
ps -efinside the container — Java is now PID 1.docker stop— graceful shutdown logs are now visible confirming the JVM received SIGTERM.Public issue : wso2/product-is#27711
Related PR: wso2/docker-is#507