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

Refresh runner uses Process.Kill() which does not allow process to exit gracefully #8

Open
saurori opened this issue Nov 17, 2023 · 0 comments

Comments

@saurori
Copy link

saurori commented Nov 17, 2023

Description

Description

When restarting a process, the runner calls cmd.Process.Kill() which immediately kills the process and does not allow it to clean up / shutdown.

This is an issue when using buffalo dev on a Buffalo application that runs a custom Server implementation. The custom server implementation has a context passed to Start() as well as a Shutdown() function. The Start() function never receives the context cancellation and Shutdown() is never called.

Expected Behavior

The process can exit and call shutdown functions properly.

Actual Behavior

The process exits immediately.

Proposed Solution

I am guessing Kill() was used because it provides the same behavior between unix and windows. However, I think a combined solution using cmd.Process.Signal(os.Interrupt) and if that returns error, run Kill(). There could also be a timeout when calling Interrupt, in case it hangs.

The Signal package states:

On Windows, sending os.Interrupt to a process with os.Process.Signal is not implemented; it will return an error instead of sending a signal.

I can try to put together a pull request, but wanted to get feedback first.

To Reproduce

  1. Implement interface server.Server
  2. Use custom server along main app, e.g: app.Serve(customServer, servers.New())
  3. In Start(ctx) listen for ctx.Done or log in Shutdown()

Additional Context

Details

ProductName:		macOS
ProductVersion:		13.5.2
BuildVersion:		22G91

-> Go: Checking minimum version requirements
✓ Your version of Go, 1.21.4, meets the minimum requirements.

-> Buffalo (CLI): Checking minimum version requirements
✓ Your version of Buffalo (CLI), v0.18.14, meets the minimum requirements.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant