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

[Fixes #197] Gracefully shutdown HTTP server #201

Merged
merged 2 commits into from
Jul 12, 2024

Conversation

dcarley
Copy link
Contributor

@dcarley dcarley commented Jul 12, 2024

To prevent a race condition seen in #197:

  1. On the client:
    a. User runs process-compose down
    b. Client makes request to /project/stop
  2. On the server:
    a. ProjectRunner.ShutDownProject() stops all processes
    b. cmd.runProject() stops blocking
    c. Exits before PcApi.ShutDownProject has written the response
  3. On the client:
    a. Gets EOF reading the response because the server has gone away

I can't think of a way to reproduce this in a test, except for something
convoluted like creating a custom client that introduces a delay in
reading the response.

It can be "manually" simulated by adding a small delay here though:

diff --git a/src/api/pc_api.go b/src/api/pc_api.go
index d123257..d069399 100644
--- a/src/api/pc_api.go
+++ b/src/api/pc_api.go
@@ -3,6 +3,7 @@ package api
 import (
 	"net/http"
 	"strconv"
+	"time"
 
 	"github.com/f1bonacc1/process-compose/src/app"
 	"github.com/gin-gonic/gin"
@@ -268,6 +269,7 @@ func (api *PcApi) GetProcessPorts(c *gin.Context) {
 // @Router /project/stop [post]
 func (api *PcApi) ShutDownProject(c *gin.Context) {
 	api.project.ShutDownProject()
+	time.Sleep(10 * time.Millisecond)
 	c.JSON(http.StatusOK, gin.H{"status": "stopped"})
 }

And using this config:

processes:
  one:
    command: sleep infinity
  two:
    command: sleep infinity

Before this change:

bash-5.2$ go run src/main.go up --config services.yaml --tui=false &
[1] 8171
bash-5.2$ go run src/main.go down
24-07-12 15:38:40.332 FTL failed to stop project error="Post \"http://localhost:8080/project/stop\": EOF"
exit status 1
[1]+  Done                    go run src/main.go up --config services.yaml --tui=false

After this change:

bash-5.2$ go run src/main.go up --config services.yaml --tui=false &
[1] 8432
bash-5.2$ go run src/main.go down

dcarley added 2 commits July 12, 2024 15:01
In preparation for being able to gracefully shutdown the HTTP server.

The existing exit code is wrapped in a new type that satisfies `error`
and lets us pull the code out for `os.Exit`.
To prevent a race condition seen in F1bonacc1#197:

1. On the client:
  a. User runs `process-compose down`
  b. Client makes request to `/project/stop`
2. On the server:
  a. `ProjectRunner.ShutDownProject()` stops all processes
  b. `cmd.runProject()` stops blocking
  c. Exits before `PcApi.ShutDownProject` has written the response
3. On the client:
  a. Gets `EOF` reading the response because the server has gone away

I can't think of a way to reproduce this in a test, except for something
convoluted like creating a custom client that introduces a delay in
reading the response.

It can be "manually" simulated by adding a small delay here though:

    diff --git a/src/api/pc_api.go b/src/api/pc_api.go
    index d123257..d069399 100644
    --- a/src/api/pc_api.go
    +++ b/src/api/pc_api.go
    @@ -3,6 +3,7 @@ package api
     import (
 	    "net/http"
 	    "strconv"
    +	"time"

 	    "github.com/f1bonacc1/process-compose/src/app"
 	    "github.com/gin-gonic/gin"
    @@ -268,6 +269,7 @@ func (api *PcApi) GetProcessPorts(c *gin.Context) {
     // @router /project/stop [post]
     func (api *PcApi) ShutDownProject(c *gin.Context) {
 	    api.project.ShutDownProject()
    +	time.Sleep(10 * time.Millisecond)
 	    c.JSON(http.StatusOK, gin.H{"status": "stopped"})
     }

And using this config:

    processes:
      one:
        command: sleep infinity
      two:
        command: sleep infinity

Before this change:

    bash-5.2$ go run src/main.go up --config services.yaml --tui=false &
    [1] 8171
    bash-5.2$ go run src/main.go down
    24-07-12 15:38:40.332 FTL failed to stop project error="Post \"http://localhost:8080/project/stop\": EOF"
    exit status 1
    [1]+  Done                    go run src/main.go up --config services.yaml --tui=false

After this change:

    bash-5.2$ go run src/main.go up --config services.yaml --tui=false &
    [1] 8432
    bash-5.2$ go run src/main.go down
Copy link
Owner

@F1bonacc1 F1bonacc1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That looks really good!
Thanks for fixing this so quickly!

@F1bonacc1 F1bonacc1 merged commit 07947a7 into F1bonacc1:main Jul 12, 2024
2 checks passed
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

Successfully merging this pull request may close these issues.

2 participants