Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

jetty.sh does not stop jetty anymore #10271

Closed
l-cdc opened this issue Aug 8, 2023 · 5 comments · Fixed by #10272
Closed

jetty.sh does not stop jetty anymore #10271

l-cdc opened this issue Aug 8, 2023 · 5 comments · Fixed by #10272
Assignees
Labels
Bug For general bugs on Jetty side

Comments

@l-cdc
Copy link

l-cdc commented Aug 8, 2023

Jetty version(s)
10.0.15

Java version/vendor (use: java -version)
openjdk version "11.0.19" 2023-04-18 LTS
OpenJDK Runtime Environment (Red_Hat-11.0.19.0.7-1.el7_9) (build 11.0.19+7-LTS)
OpenJDK 64-Bit Server VM (Red_Hat-11.0.19.0.7-1.el7_9) (build 11.0.19+7-LTS, mixed mode, sharing)

OS type/version
Red Hat Enterprise Linux Server release 7.9 (Maipo)

Description
This bug was likely introduced by #9309 which introduces xargs to launch java. The relevant lines are

        echo ${RUN_ARGS[*]} | xargs ${JAVA} > /dev/null &
        disown $!
        echo $! > "$JETTY_PID"

$! now returns the PID of the xargs process, not the java one. Consequently, jetty.sh stop now stops xargs, but at least on my system killing xargs (version "xargs (GNU findutils) 4.5.11") does not terminate its child processes.

Note also that on systems with start-stop-daemon, this probably works as start-stop-daemon writes the child PID in the PID file if I understand the man page correctly.

The probably simplest (not best) way to fix this is to retrieve the correct PID with something like

 echo `pgrep -P $!` > "$JETTY_PID"

How to reproduce?
Issue should appear with unmodified jetty distribution on any system without start-stop-daemon.

@l-cdc l-cdc added the Bug For general bugs on Jetty side label Aug 8, 2023
@joakime joakime self-assigned this Aug 8, 2023
@joakime
Copy link
Contributor

joakime commented Aug 8, 2023

Thank you for the detailed issue, and initial triage.

I think having Jetty itself produce the pid file is going to be the safest solution.

joakime added a commit that referenced this issue Aug 8, 2023
+ Have jetty.sh use --pid-file=<name> too

Signed-off-by: Joakim Erdfelt <[email protected]>
@joakime joakime moved this to 🏗 In progress in Jetty 10.0.16 / 11.0.16 - FROZEN Aug 8, 2023
@joakime joakime linked a pull request Aug 8, 2023 that will close this issue
@joakime
Copy link
Contributor

joakime commented Aug 8, 2023

Opened PR #10272 - @l-cdc can you give try out that branch? see if it works for you?
Meanwhile, I'll continue testing ...

@joakime
Copy link
Contributor

joakime commented Aug 8, 2023

@l-cdc what kind of environment are you running in which doesn't have start-stop-daemon ?
I'm having a hard time finding one to test in.

@joakime
Copy link
Contributor

joakime commented Aug 8, 2023

Ah! there we go ...

$ docker run --rm -it redhat/ubi8:latest bash
Unable to find image 'redhat/ubi8:latest' locally
latest: Pulling from redhat/ubi8
0fa65fe5c23e: Pull complete 
Digest: sha256:a7143118671dfc61aca46e8ab9e488500495a3c4c73a69577ca9386564614c13
Status: Downloaded newer image for redhat/ubi8:latest
[root@79aca762789a /]# which start-stop-daemon
/usr/bin/which: no start-stop-daemon in (/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin)
[root@79aca762789a /]# 

@l-cdc
Copy link
Author

l-cdc commented Aug 16, 2023

@joakime I tested the branch fix/10.0.x/start-pidfile and it works for me. Many thanks for the very rapid response and PR!

@github-project-automation github-project-automation bot moved this from 🏗 In progress to ✅ Done in Jetty 10.0.16 / 11.0.16 - FROZEN Aug 25, 2023
joakime added a commit that referenced this issue Aug 25, 2023
* Issue #10271 - new jetty-home module `pid`

Signed-off-by: Joakim Erdfelt <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug For general bugs on Jetty side
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants