fix: send SIGTERM signal to --cmd instead of SIGKILL

[guilhas07]

Problem

When trying to use the command templ generate --watch --proxy="http://localhost:8000" --cmd="wgo run main.go" I noticed that wgo didn't close correctly. After discussing on the wgo repo ( see here ) the conclusion was that this was due to templ sending a SIGKILL signal not allowing for wgo to close correctly.

Solution

To fix this I changed all SIGKILL signals to SIGTERM.

Caveats:

• I don't know if this is a problem too on windows
• I suppose this doesn't break any behaviour, but let me know if it does.

Let me know if there is something I need to change!

[a-h]

Thanks, I'll have a read through of the other repo. Sigkill kills child processes, where sigterm doesn't, so there's a good reason to use kill - for example, if you do go run, it does a build, then runs the executable as a child process, so sigterm wouldn't necessarily kill that process.

[guilhas07]

Thanks, I'll have a read through of the other repo. Sigkill kills child processes, where sigterm doesn't, so there's a good reason to use kill - for example, if you do go run, it does a build, then runs the executable as a child process, so sigterm wouldn't necessarily kill that process.

Using go run seems to be working as expected with this version too. Using this example bokwoon95/wgo#8 (comment) from the issue above.

Isn't the propagation of the signal dependent on the process group id, so even if we use SIGKILL, we need to send to the group id so that the children will terminate https://stackoverflow.com/questions/21869242/killing-parent-process-along-with-child-process-using-sigkill . So by using SIGTERM it would still be fine if we send to the process group id, right?

[a-h]

Reading through Air's code, it appears that it can send a SIGTERM to initiate graceful shutdown, prior to attempting to forcefully kill the process after a timeout: https://github.com/cosmtrek/air/blob/3f19370fe5e8d8fe2ddd927de54b1aede379f2b7/runner/util_unix.go#L13

I think that's what templ needs to do in order to allow wgo to shutdown any processes it's started gracefully.

[guilhas07]

Reading through Air's code, it appears that it can send a SIGTERM to initiate graceful shutdown, prior to attempting to forcefully kill the process after a timeout: https://github.com/cosmtrek/air/blob/3f19370fe5e8d8fe2ddd927de54b1aede379f2b7/runner/util_unix.go#L13

I think that's what templ needs to do in order to allow wgo to shutdown any processes it's started gracefully.

I believe the current way in this PR should be correct because we should not assume a timeout for how long the user command takes in order to execute its clean-up logic. ref: https://stackoverflow.com/a/690631/12458551 .

Edit: Maybe in this context, it would be actually useful to have a timeout anyways in favour of a better user experience in variable scenarios.

[a-h]

Hi, it took me a while to get to this. I've added a test which I think demonstrates the behaviour we're looking for.

Is this OK for you?

[guilhas07]

Hi, it took me a while to get to this. I've added a test which I think demonstrates the behaviour we're looking for.

Is this OK for you?

No worries! Working well on my end.
Thank you for looking into this!

[joerdav]

Looks good to me!