jamesfredley commented on PR #15698:
URL: https://github.com/apache/grails-core/pull/15698#issuecomment-4618837954

   @jdaugherty thanks for the review - you were right, the in-memory registry 
only worked within a single CLI JVM, so it broke for forked processes and for 
an independent `grails stop-app`. I've reworked the PR to your suggestion: 
**write the application PID during `run-app` and have `stop-app` signal that 
PID directly.**
   
   ### What changed (pushed in a53817cc4a)
   
   **Writing the PID (`run-app` side)**
   - The forked `bootRun` application writes its own PID via Spring Boot's 
`ApplicationPidFileWriter` to `build/run-app.pid`. It's registered in 
`GrailsApp.run()` only when the `cli.pid.file` system property is present, so 
normally deployed apps are unaffected.
   - `run-app` passes that path as `-Dgrails.cli.pid.file=<abs path>`; the 
`grails.` prefix is stripped by `GrailsGradlePlugin` when it forwards 
properties into the forked JVM (same mechanism already used for 
`grails.server.port` -> `server.port`), so no Gradle plugin change was needed.
   
   **Stopping by PID (`stop-app` side)**
   - New `RunningApplicationProcess` helper reads `build/run-app.pid` and stops 
the process via `ProcessHandle.of(pid).destroy()` - i.e. a graceful `SIGTERM` 
on Unix-like systems (JVM shutdown hooks / Spring orderly shutdown run), 
falling back to `destroyForcibly()` if it doesn't exit within the timeout. On 
Windows there is no graceful signal equivalent, so termination there is best 
effort (documented).
   - Includes a stale/recycled-PID guard (compares the live process 
`startInstant` against the PID file mtime) so it never kills an unrelated 
process that happens to have reused the id.
   
   Because the contract is a file on disk, this now works for the two cases you 
called out: **forked processes** and **`grails stop-app` run independently of 
the original `run-app`** (including from a different terminal).
   
   **Other details**
   - A small `run-app.stopping` marker lets a *foreground* `run-app` (blocked 
on `bootRun`) report a clean "Application stopped" instead of a startup failure 
when its process is killed by `SIGTERM` (non-zero exit). It's cleared at the 
start of each `run-app` so it can't mask a genuine startup failure.
   - A guard prevents a second `run-app` from orphaning an already-running app.
   - Reverted the `GradleUtil`/`GradleInvoker` cancellation-token changes and 
removed `RunningApplicationRegistry`; the JMX/Actuator code stays gone.
   - New `RunningApplicationProcessSpec` (13 tests: 
missing/malformed/non-positive/stale PID, live-process stop, marker lifecycle) 
plus updated docs.
   
   I kept `kill -TERM`-equivalent semantics exactly as you described. Let me 
know if you'd prefer the PID file somewhere other than `build/`, or a different 
policy for the Windows best-effort path.
   


-- 
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]

Reply via email to