From 137da86daf5bbf6eea4b927c0bdb33090557ee00 Mon Sep 17 00:00:00 2001 From: "yingjie.huang" Date: Thu, 10 Oct 2024 16:44:41 +0800 Subject: [PATCH 01/21] feat: Optimize ShutdownWithContext method in app.go - Reorder mutex lock acquisition to the start of the function - Early return if server is not running - Use defer for executing shutdown hooks - Simplify nil check for hooks - Remove TODO comment This commit improves the readability, robustness, and execution order of the shutdown process. It ensures consistent state throughout the shutdown and guarantees hook execution even in error cases. --- app.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/app.go b/app.go index e0240d3c16..e3f91cddfd 100644 --- a/app.go +++ b/app.go @@ -841,16 +841,18 @@ func (app *App) ShutdownWithTimeout(timeout time.Duration) error { // // ShutdownWithContext does not close keepalive connections so its recommended to set ReadTimeout to something else than 0. func (app *App) ShutdownWithContext(ctx context.Context) error { - if app.hooks != nil { - // TODO: check should be defered? - app.hooks.executeOnShutdownHooks() - } - app.mutex.Lock() defer app.mutex.Unlock() + if app.server == nil { return ErrNotRunning } + + // Execute shutdown hooks in a deferred function + if app.hooks != nil { + defer app.hooks.executeOnShutdownHooks() + } + return app.server.ShutdownWithContext(ctx) } From b41d084c480aa36ac6e3eaf829e65f6def255ed1 Mon Sep 17 00:00:00 2001 From: "yingjie.huang" Date: Thu, 10 Oct 2024 17:14:30 +0800 Subject: [PATCH 02/21] feat: Enhance ShutdownWithContext test for improved reliability - Add shutdown hook verification - Implement better synchronization with channels - Improve error handling and assertions - Adjust timeouts for more consistent results - Add server state check after shutdown attempt - Include comments explaining expected behavior This commit improves the comprehensiveness and reliability of the ShutdownWithContext test, ensuring proper verification of shutdown hooks, timeout behavior, and server state during long-running requests. --- app_test.go | 72 ++++++++++++++++++++++++++++++++--------------------- 1 file changed, 44 insertions(+), 28 deletions(-) diff --git a/app_test.go b/app_test.go index 6b493de1eb..f4feca9489 100644 --- a/app_test.go +++ b/app_test.go @@ -860,6 +860,12 @@ func Test_App_ShutdownWithContext(t *testing.T) { t.Parallel() app := New() + shutdownHookCalled := false + app.Hooks().OnShutdown(func() error { + shutdownHookCalled = true + return nil + }) + app.Get("/", func(ctx Ctx) error { time.Sleep(5 * time.Second) return ctx.SendString("body") @@ -867,38 +873,48 @@ func Test_App_ShutdownWithContext(t *testing.T) { ln := fasthttputil.NewInmemoryListener() - go func() { - err := app.Listener(ln) - assert.NoError(t, err) - }() - - time.Sleep(1 * time.Second) - - go func() { - conn, err := ln.Dial() - assert.NoError(t, err) - - _, err = conn.Write([]byte("GET / HTTP/1.1\r\nHost: google.com\r\n\r\n")) - assert.NoError(t, err) - }() + serverErr := make(chan error, 1) + go func() { + serverErr <- app.Listener(ln) + }() + + time.Sleep(100 * time.Millisecond) + + clientDone := make(chan struct{}) + go func() { + conn, err := ln.Dial() + assert.NoError(t, err) + _, err = conn.Write([]byte("GET / HTTP/1.1\r\nHost: example.com\r\n\r\n")) + assert.NoError(t, err) + close(clientDone) + }() + + <-clientDone + time.Sleep(100 * time.Millisecond) + + shutdownErr := make(chan error, 1) + go func() { + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) + defer cancel() + shutdownErr <- app.ShutdownWithContext(ctx) + }() - time.Sleep(1 * time.Second) + select { + case <-time.After(2 * time.Second): + t.Fatal("shutdown did not complete in time") + case err := <-shutdownErr: + assert.Error(t, err, "Expected shutdown to return an error due to timeout") + assert.True(t, errors.Is(err, context.DeadlineExceeded), "Expected DeadlineExceeded error") + } - shutdownErr := make(chan error) - go func() { - ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) - defer cancel() - shutdownErr <- app.ShutdownWithContext(ctx) - }() + assert.True(t, shutdownHookCalled, "Shutdown hook was not called") select { - case <-time.After(5 * time.Second): - t.Fatal("idle connections not closed on shutdown") - case err := <-shutdownErr: - if err == nil || !errors.Is(err, context.DeadlineExceeded) { - t.Fatalf("unexpected err %v. Expecting %v", err, context.DeadlineExceeded) - } - } + case err := <-serverErr: + assert.NoError(t, err, "Server should have shut down without error") + default: + // Server is still running, which is expected as the long-running request prevented full shutdown + } } // go test -run Test_App_Mixed_Routes_WithSameLen From a6831665f6329835728fb06a81f6acff97814617 Mon Sep 17 00:00:00 2001 From: JIeJaitt <498938874@qq.com> Date: Sat, 12 Oct 2024 23:06:41 +0800 Subject: [PATCH 03/21] =?UTF-8?q?=F0=9F=93=9A=20Doc:=20update=20the=20docs?= =?UTF-8?q?=20to=20explain=20shutdown=20&=20hook=20execution=20order?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/api/fiber.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/api/fiber.md b/docs/api/fiber.md index 16f23b9e61..73878c61aa 100644 --- a/docs/api/fiber.md +++ b/docs/api/fiber.md @@ -205,7 +205,7 @@ Shutdown gracefully shuts down the server without interrupting any active connec ShutdownWithTimeout will forcefully close any active connections after the timeout expires. -ShutdownWithContext shuts down the server including by force if the context's deadline is exceeded. +ShutdownWithContext shuts down the server including by force if the context's deadline is exceeded.Shutdown hooks will still be executed, even if an error occurs during the shutdown process, as they are deferred to ensure cleanup happens regardless of errors. ```go func (app *App) Shutdown() error From 796922faf2ad4c0e2728e46e28c6cff8969d6ff1 Mon Sep 17 00:00:00 2001 From: JIeJaitt <498938874@qq.com> Date: Sun, 13 Oct 2024 00:24:59 +0800 Subject: [PATCH 04/21] =?UTF-8?q?=F0=9F=A9=B9=20Fix:=20Possible=20Data=20R?= =?UTF-8?q?ace=20on=20shutdownHookCalled=20Variable?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app_test.go b/app_test.go index f4feca9489..20286d34fa 100644 --- a/app_test.go +++ b/app_test.go @@ -860,9 +860,9 @@ func Test_App_ShutdownWithContext(t *testing.T) { t.Parallel() app := New() - shutdownHookCalled := false + var shutdownHookCalled int32 app.Hooks().OnShutdown(func() error { - shutdownHookCalled = true + atomic.StoreInt32(&shutdownHookCalled, 1) return nil }) @@ -907,7 +907,7 @@ func Test_App_ShutdownWithContext(t *testing.T) { assert.True(t, errors.Is(err, context.DeadlineExceeded), "Expected DeadlineExceeded error") } - assert.True(t, shutdownHookCalled, "Shutdown hook was not called") + assert.Equal(t, int32(1), atomic.LoadInt32(&shutdownHookCalled), "Shutdown hook was not called") select { case err := <-serverErr: From e465b5bd176912d3f421f263c317d6827935da19 Mon Sep 17 00:00:00 2001 From: JIeJaitt <498938874@qq.com> Date: Sun, 13 Oct 2024 00:27:31 +0800 Subject: [PATCH 05/21] =?UTF-8?q?=F0=9F=A9=B9=20Fix:=20Remove=20the=20defa?= =?UTF-8?q?ult=20Case?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app_test.go b/app_test.go index 20286d34fa..49daaf99c8 100644 --- a/app_test.go +++ b/app_test.go @@ -912,7 +912,7 @@ func Test_App_ShutdownWithContext(t *testing.T) { select { case err := <-serverErr: assert.NoError(t, err, "Server should have shut down without error") - default: + // default: // Server is still running, which is expected as the long-running request prevented full shutdown } } From ee866ecbe615937b642f9af71c6209bbe58779b5 Mon Sep 17 00:00:00 2001 From: JIeJaitt <498938874@qq.com> Date: Sun, 13 Oct 2024 00:48:55 +0800 Subject: [PATCH 06/21] =?UTF-8?q?=F0=9F=A9=B9=20Fix:=20Import=20sync/atomi?= =?UTF-8?q?c?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/app_test.go b/app_test.go index 49daaf99c8..359d201947 100644 --- a/app_test.go +++ b/app_test.go @@ -22,6 +22,7 @@ import ( "strings" "testing" "time" + "sync/atomic" "github.com/gofiber/utils/v2" From e0a56bef5fa3bea3546ea2f77d5f275ba8eb85ca Mon Sep 17 00:00:00 2001 From: JIeJaitt <498938874@qq.com> Date: Sun, 13 Oct 2024 19:49:29 +0800 Subject: [PATCH 07/21] =?UTF-8?q?=F0=9F=A9=B9=20Fix:=20golangci-lint=20pro?= =?UTF-8?q?blem?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app_test.go | 81 ++++++++++++++++++++++++++--------------------------- go.mod | 1 + go.sum | 2 ++ 3 files changed, 43 insertions(+), 41 deletions(-) diff --git a/app_test.go b/app_test.go index 359d201947..e23bca57b2 100644 --- a/app_test.go +++ b/app_test.go @@ -20,9 +20,9 @@ import ( "regexp" "runtime" "strings" + "sync/atomic" "testing" "time" - "sync/atomic" "github.com/gofiber/utils/v2" @@ -861,11 +861,11 @@ func Test_App_ShutdownWithContext(t *testing.T) { t.Parallel() app := New() - var shutdownHookCalled int32 - app.Hooks().OnShutdown(func() error { - atomic.StoreInt32(&shutdownHookCalled, 1) - return nil - }) + var shutdownHookCalled atomic.Int32 + app.Hooks().OnShutdown(func() error { + shutdownHookCalled.Store(1) + return nil + }) app.Get("/", func(ctx Ctx) error { time.Sleep(5 * time.Second) @@ -874,48 +874,47 @@ func Test_App_ShutdownWithContext(t *testing.T) { ln := fasthttputil.NewInmemoryListener() - serverErr := make(chan error, 1) - go func() { - serverErr <- app.Listener(ln) - }() - - time.Sleep(100 * time.Millisecond) - - clientDone := make(chan struct{}) - go func() { - conn, err := ln.Dial() - assert.NoError(t, err) - _, err = conn.Write([]byte("GET / HTTP/1.1\r\nHost: example.com\r\n\r\n")) - assert.NoError(t, err) - close(clientDone) - }() - + serverErr := make(chan error, 1) + go func() { + serverErr <- app.Listener(ln) + }() + + time.Sleep(100 * time.Millisecond) + + clientDone := make(chan struct{}) + go func() { + conn, err := ln.Dial() + assert.NoError(t, err) + _, err = conn.Write([]byte("GET / HTTP/1.1\r\nHost: example.com\r\n\r\n")) + assert.NoError(t, err) + close(clientDone) + }() + <-clientDone - time.Sleep(100 * time.Millisecond) + // Sleep to ensure the server has started processing the request + time.Sleep(100 * time.Millisecond) shutdownErr := make(chan error, 1) - go func() { - ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) - defer cancel() - shutdownErr <- app.ShutdownWithContext(ctx) - }() + go func() { + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) + defer cancel() + shutdownErr <- app.ShutdownWithContext(ctx) + }() select { - case <-time.After(2 * time.Second): - t.Fatal("shutdown did not complete in time") - case err := <-shutdownErr: - assert.Error(t, err, "Expected shutdown to return an error due to timeout") - assert.True(t, errors.Is(err, context.DeadlineExceeded), "Expected DeadlineExceeded error") - } + case <-time.After(2 * time.Second): + t.Fatal("shutdown did not complete in time") + case err := <-shutdownErr: + require.Error(t, err, "Expected shutdown to return an error due to timeout") + require.ErrorIs(t, err, context.DeadlineExceeded, "Expected DeadlineExceeded error") + } - assert.Equal(t, int32(1), atomic.LoadInt32(&shutdownHookCalled), "Shutdown hook was not called") + assert.Equal(t, int32(1), shutdownHookCalled.Load(), "Shutdown hook was not called") - select { - case err := <-serverErr: - assert.NoError(t, err, "Server should have shut down without error") - // default: - // Server is still running, which is expected as the long-running request prevented full shutdown - } + err := <-serverErr + assert.NoError(t, err, "Server should have shut down without error") + // default: + // Server is still running, which is expected as the long-running request prevented full shutdown } // go test -run Test_App_Mixed_Routes_WithSameLen diff --git a/go.mod b/go.mod index 8f3a2a43d3..4b15327e8a 100644 --- a/go.mod +++ b/go.mod @@ -21,6 +21,7 @@ require ( github.com/philhofer/fwd v1.1.3-0.20240612014219-fbbf4953d986 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/valyala/tcplisten v1.0.0 // indirect + golang.org/dl v0.0.0-20241001165935-bedb0f791d00 // indirect golang.org/x/net v0.29.0 // indirect golang.org/x/sys v0.25.0 // indirect golang.org/x/text v0.18.0 // indirect diff --git a/go.sum b/go.sum index 1d53c4560b..a8c6f97f42 100644 --- a/go.sum +++ b/go.sum @@ -29,6 +29,8 @@ github.com/valyala/fasthttp v1.56.0 h1:bEZdJev/6LCBlpdORfrLu/WOZXXxvrUQSiyniuaoW github.com/valyala/fasthttp v1.56.0/go.mod h1:sReBt3XZVnudxuLOx4J/fMrJVorWRiWY2koQKgABiVI= github.com/valyala/tcplisten v1.0.0 h1:rBHj/Xf+E1tRGZyWIWwJDiRY0zc1Js+CV5DqwacVSA8= github.com/valyala/tcplisten v1.0.0/go.mod h1:T0xQ8SeCZGxckz9qRXTfG43PvQ/mcWh7FwZEA7Ioqkc= +golang.org/dl v0.0.0-20241001165935-bedb0f791d00 h1:OX0WPBB1pQPZy1SL0+q5C/VuuM6e1wv6uEuB9iyBi/I= +golang.org/dl v0.0.0-20241001165935-bedb0f791d00/go.mod h1:fwQ+hlTD8I6TIzOGkQqxQNfE2xqR+y7SzGaDkksVFkw= golang.org/x/net v0.29.0 h1:5ORfpBpCs4HzDYoodCDBbwHzdR5UrLBZ3sOnUJmFoHo= golang.org/x/net v0.29.0/go.mod h1:gLkgy8jTGERgjzMic6DS9+SP0ajcu6Xu3Orq/SpETg0= golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= From 750a7facb922d11207a1c8fe5f4604eeb0a67ef8 Mon Sep 17 00:00:00 2001 From: JIeJaitt <498938874@qq.com> Date: Mon, 28 Oct 2024 14:25:29 +0800 Subject: [PATCH 08/21] =?UTF-8?q?=F0=9F=8E=A8=20Style:=20add=20block=20in?= =?UTF-8?q?=20api.md?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/api/fiber.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/api/fiber.md b/docs/api/fiber.md index 73878c61aa..a0ab078776 100644 --- a/docs/api/fiber.md +++ b/docs/api/fiber.md @@ -205,7 +205,7 @@ Shutdown gracefully shuts down the server without interrupting any active connec ShutdownWithTimeout will forcefully close any active connections after the timeout expires. -ShutdownWithContext shuts down the server including by force if the context's deadline is exceeded.Shutdown hooks will still be executed, even if an error occurs during the shutdown process, as they are deferred to ensure cleanup happens regardless of errors. +ShutdownWithContext shuts down the server including by force if the context's deadline is exceeded. Shutdown hooks will still be executed, even if an error occurs during the shutdown process, as they are deferred to ensure cleanup happens regardless of errors. ```go func (app *App) Shutdown() error From b9509fe4d2b4b7e97f1d98970e0ece3537616372 Mon Sep 17 00:00:00 2001 From: JIeJaitt <498938874@qq.com> Date: Mon, 28 Oct 2024 16:37:52 +0800 Subject: [PATCH 09/21] =?UTF-8?q?=F0=9F=A9=B9=20Fix:=20go=20mod=20tidy?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- go.mod | 1 - go.sum | 2 -- 2 files changed, 3 deletions(-) diff --git a/go.mod b/go.mod index dd7324b3f0..2b5e60a1bf 100644 --- a/go.mod +++ b/go.mod @@ -21,7 +21,6 @@ require ( github.com/philhofer/fwd v1.1.3-0.20240612014219-fbbf4953d986 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/valyala/tcplisten v1.0.0 // indirect - golang.org/dl v0.0.0-20241001165935-bedb0f791d00 // indirect golang.org/x/net v0.29.0 // indirect golang.org/x/sys v0.25.0 // indirect golang.org/x/text v0.18.0 // indirect diff --git a/go.sum b/go.sum index fd50cfe1c0..42768451af 100644 --- a/go.sum +++ b/go.sum @@ -33,8 +33,6 @@ github.com/valyala/tcplisten v1.0.0 h1:rBHj/Xf+E1tRGZyWIWwJDiRY0zc1Js+CV5DqwacVS github.com/valyala/tcplisten v1.0.0/go.mod h1:T0xQ8SeCZGxckz9qRXTfG43PvQ/mcWh7FwZEA7Ioqkc= github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM= github.com/x448/float16 v0.8.4/go.mod h1:14CWIYCyZA/cWjXOioeEpHeN/83MdbZDRQHoFcYsOfg= -golang.org/dl v0.0.0-20241001165935-bedb0f791d00 h1:OX0WPBB1pQPZy1SL0+q5C/VuuM6e1wv6uEuB9iyBi/I= -golang.org/dl v0.0.0-20241001165935-bedb0f791d00/go.mod h1:fwQ+hlTD8I6TIzOGkQqxQNfE2xqR+y7SzGaDkksVFkw= golang.org/x/net v0.29.0 h1:5ORfpBpCs4HzDYoodCDBbwHzdR5UrLBZ3sOnUJmFoHo= golang.org/x/net v0.29.0/go.mod h1:gLkgy8jTGERgjzMic6DS9+SP0ajcu6Xu3Orq/SpETg0= golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= From c2792e7bee78600918d34709af731f120f647058 Mon Sep 17 00:00:00 2001 From: "yingjie.huang" Date: Thu, 10 Oct 2024 16:44:41 +0800 Subject: [PATCH 10/21] feat: Optimize ShutdownWithContext method in app.go - Reorder mutex lock acquisition to the start of the function - Early return if server is not running - Use defer for executing shutdown hooks - Simplify nil check for hooks - Remove TODO comment This commit improves the readability, robustness, and execution order of the shutdown process. It ensures consistent state throughout the shutdown and guarantees hook execution even in error cases. --- app.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/app.go b/app.go index 38a3d17319..a562cc7f0a 100644 --- a/app.go +++ b/app.go @@ -878,16 +878,18 @@ func (app *App) ShutdownWithTimeout(timeout time.Duration) error { // // ShutdownWithContext does not close keepalive connections so its recommended to set ReadTimeout to something else than 0. func (app *App) ShutdownWithContext(ctx context.Context) error { - if app.hooks != nil { - // TODO: check should be defered? - app.hooks.executeOnShutdownHooks() - } - app.mutex.Lock() defer app.mutex.Unlock() + if app.server == nil { return ErrNotRunning } + + // Execute shutdown hooks in a deferred function + if app.hooks != nil { + defer app.hooks.executeOnShutdownHooks() + } + return app.server.ShutdownWithContext(ctx) } From 18111e55d13ee38111fbb5deb61bc62a602026ed Mon Sep 17 00:00:00 2001 From: "yingjie.huang" Date: Thu, 10 Oct 2024 17:14:30 +0800 Subject: [PATCH 11/21] feat: Enhance ShutdownWithContext test for improved reliability - Add shutdown hook verification - Implement better synchronization with channels - Improve error handling and assertions - Adjust timeouts for more consistent results - Add server state check after shutdown attempt - Include comments explaining expected behavior This commit improves the comprehensiveness and reliability of the ShutdownWithContext test, ensuring proper verification of shutdown hooks, timeout behavior, and server state during long-running requests. --- app_test.go | 72 ++++++++++++++++++++++++++++++++--------------------- 1 file changed, 44 insertions(+), 28 deletions(-) diff --git a/app_test.go b/app_test.go index 6b493de1eb..f4feca9489 100644 --- a/app_test.go +++ b/app_test.go @@ -860,6 +860,12 @@ func Test_App_ShutdownWithContext(t *testing.T) { t.Parallel() app := New() + shutdownHookCalled := false + app.Hooks().OnShutdown(func() error { + shutdownHookCalled = true + return nil + }) + app.Get("/", func(ctx Ctx) error { time.Sleep(5 * time.Second) return ctx.SendString("body") @@ -867,38 +873,48 @@ func Test_App_ShutdownWithContext(t *testing.T) { ln := fasthttputil.NewInmemoryListener() - go func() { - err := app.Listener(ln) - assert.NoError(t, err) - }() - - time.Sleep(1 * time.Second) - - go func() { - conn, err := ln.Dial() - assert.NoError(t, err) - - _, err = conn.Write([]byte("GET / HTTP/1.1\r\nHost: google.com\r\n\r\n")) - assert.NoError(t, err) - }() + serverErr := make(chan error, 1) + go func() { + serverErr <- app.Listener(ln) + }() + + time.Sleep(100 * time.Millisecond) + + clientDone := make(chan struct{}) + go func() { + conn, err := ln.Dial() + assert.NoError(t, err) + _, err = conn.Write([]byte("GET / HTTP/1.1\r\nHost: example.com\r\n\r\n")) + assert.NoError(t, err) + close(clientDone) + }() + + <-clientDone + time.Sleep(100 * time.Millisecond) + + shutdownErr := make(chan error, 1) + go func() { + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) + defer cancel() + shutdownErr <- app.ShutdownWithContext(ctx) + }() - time.Sleep(1 * time.Second) + select { + case <-time.After(2 * time.Second): + t.Fatal("shutdown did not complete in time") + case err := <-shutdownErr: + assert.Error(t, err, "Expected shutdown to return an error due to timeout") + assert.True(t, errors.Is(err, context.DeadlineExceeded), "Expected DeadlineExceeded error") + } - shutdownErr := make(chan error) - go func() { - ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) - defer cancel() - shutdownErr <- app.ShutdownWithContext(ctx) - }() + assert.True(t, shutdownHookCalled, "Shutdown hook was not called") select { - case <-time.After(5 * time.Second): - t.Fatal("idle connections not closed on shutdown") - case err := <-shutdownErr: - if err == nil || !errors.Is(err, context.DeadlineExceeded) { - t.Fatalf("unexpected err %v. Expecting %v", err, context.DeadlineExceeded) - } - } + case err := <-serverErr: + assert.NoError(t, err, "Server should have shut down without error") + default: + // Server is still running, which is expected as the long-running request prevented full shutdown + } } // go test -run Test_App_Mixed_Routes_WithSameLen From 83ea43d4f688ec4f961e00b5983c116133ce2d97 Mon Sep 17 00:00:00 2001 From: JIeJaitt <498938874@qq.com> Date: Sat, 12 Oct 2024 23:06:41 +0800 Subject: [PATCH 12/21] =?UTF-8?q?=F0=9F=93=9A=20Doc:=20update=20the=20docs?= =?UTF-8?q?=20to=20explain=20shutdown=20&=20hook=20execution=20order?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/api/fiber.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/api/fiber.md b/docs/api/fiber.md index 6892225e11..4cb1e0a77c 100644 --- a/docs/api/fiber.md +++ b/docs/api/fiber.md @@ -205,7 +205,7 @@ Shutdown gracefully shuts down the server without interrupting any active connec ShutdownWithTimeout will forcefully close any active connections after the timeout expires. -ShutdownWithContext shuts down the server including by force if the context's deadline is exceeded. +ShutdownWithContext shuts down the server including by force if the context's deadline is exceeded.Shutdown hooks will still be executed, even if an error occurs during the shutdown process, as they are deferred to ensure cleanup happens regardless of errors. ```go func (app *App) Shutdown() error From 66dcb42ec96dff22439b20c9438e00a46ae7664d Mon Sep 17 00:00:00 2001 From: JIeJaitt <498938874@qq.com> Date: Sun, 13 Oct 2024 00:24:59 +0800 Subject: [PATCH 13/21] =?UTF-8?q?=F0=9F=A9=B9=20Fix:=20Possible=20Data=20R?= =?UTF-8?q?ace=20on=20shutdownHookCalled=20Variable?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app_test.go b/app_test.go index f4feca9489..20286d34fa 100644 --- a/app_test.go +++ b/app_test.go @@ -860,9 +860,9 @@ func Test_App_ShutdownWithContext(t *testing.T) { t.Parallel() app := New() - shutdownHookCalled := false + var shutdownHookCalled int32 app.Hooks().OnShutdown(func() error { - shutdownHookCalled = true + atomic.StoreInt32(&shutdownHookCalled, 1) return nil }) @@ -907,7 +907,7 @@ func Test_App_ShutdownWithContext(t *testing.T) { assert.True(t, errors.Is(err, context.DeadlineExceeded), "Expected DeadlineExceeded error") } - assert.True(t, shutdownHookCalled, "Shutdown hook was not called") + assert.Equal(t, int32(1), atomic.LoadInt32(&shutdownHookCalled), "Shutdown hook was not called") select { case err := <-serverErr: From f3902c5729f4f40dc1c4bc71e7e6cc1809b8b33d Mon Sep 17 00:00:00 2001 From: JIeJaitt <498938874@qq.com> Date: Sun, 13 Oct 2024 00:27:31 +0800 Subject: [PATCH 14/21] =?UTF-8?q?=F0=9F=A9=B9=20Fix:=20Remove=20the=20defa?= =?UTF-8?q?ult=20Case?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app_test.go b/app_test.go index 20286d34fa..49daaf99c8 100644 --- a/app_test.go +++ b/app_test.go @@ -912,7 +912,7 @@ func Test_App_ShutdownWithContext(t *testing.T) { select { case err := <-serverErr: assert.NoError(t, err, "Server should have shut down without error") - default: + // default: // Server is still running, which is expected as the long-running request prevented full shutdown } } From da193ac1037063304f73c51424972f5df22f20ef Mon Sep 17 00:00:00 2001 From: JIeJaitt <498938874@qq.com> Date: Sun, 13 Oct 2024 00:48:55 +0800 Subject: [PATCH 15/21] =?UTF-8?q?=F0=9F=A9=B9=20Fix:=20Import=20sync/atomi?= =?UTF-8?q?c?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/app_test.go b/app_test.go index 49daaf99c8..359d201947 100644 --- a/app_test.go +++ b/app_test.go @@ -22,6 +22,7 @@ import ( "strings" "testing" "time" + "sync/atomic" "github.com/gofiber/utils/v2" From 0a921254c20725c07edcbbe2b23fcc5e99c65c40 Mon Sep 17 00:00:00 2001 From: JIeJaitt <498938874@qq.com> Date: Sun, 13 Oct 2024 19:49:29 +0800 Subject: [PATCH 16/21] =?UTF-8?q?=F0=9F=A9=B9=20Fix:=20golangci-lint=20pro?= =?UTF-8?q?blem?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app_test.go | 81 ++++++++++++++++++++++++++--------------------------- go.mod | 1 + go.sum | 2 ++ 3 files changed, 43 insertions(+), 41 deletions(-) diff --git a/app_test.go b/app_test.go index 359d201947..e23bca57b2 100644 --- a/app_test.go +++ b/app_test.go @@ -20,9 +20,9 @@ import ( "regexp" "runtime" "strings" + "sync/atomic" "testing" "time" - "sync/atomic" "github.com/gofiber/utils/v2" @@ -861,11 +861,11 @@ func Test_App_ShutdownWithContext(t *testing.T) { t.Parallel() app := New() - var shutdownHookCalled int32 - app.Hooks().OnShutdown(func() error { - atomic.StoreInt32(&shutdownHookCalled, 1) - return nil - }) + var shutdownHookCalled atomic.Int32 + app.Hooks().OnShutdown(func() error { + shutdownHookCalled.Store(1) + return nil + }) app.Get("/", func(ctx Ctx) error { time.Sleep(5 * time.Second) @@ -874,48 +874,47 @@ func Test_App_ShutdownWithContext(t *testing.T) { ln := fasthttputil.NewInmemoryListener() - serverErr := make(chan error, 1) - go func() { - serverErr <- app.Listener(ln) - }() - - time.Sleep(100 * time.Millisecond) - - clientDone := make(chan struct{}) - go func() { - conn, err := ln.Dial() - assert.NoError(t, err) - _, err = conn.Write([]byte("GET / HTTP/1.1\r\nHost: example.com\r\n\r\n")) - assert.NoError(t, err) - close(clientDone) - }() - + serverErr := make(chan error, 1) + go func() { + serverErr <- app.Listener(ln) + }() + + time.Sleep(100 * time.Millisecond) + + clientDone := make(chan struct{}) + go func() { + conn, err := ln.Dial() + assert.NoError(t, err) + _, err = conn.Write([]byte("GET / HTTP/1.1\r\nHost: example.com\r\n\r\n")) + assert.NoError(t, err) + close(clientDone) + }() + <-clientDone - time.Sleep(100 * time.Millisecond) + // Sleep to ensure the server has started processing the request + time.Sleep(100 * time.Millisecond) shutdownErr := make(chan error, 1) - go func() { - ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) - defer cancel() - shutdownErr <- app.ShutdownWithContext(ctx) - }() + go func() { + ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second) + defer cancel() + shutdownErr <- app.ShutdownWithContext(ctx) + }() select { - case <-time.After(2 * time.Second): - t.Fatal("shutdown did not complete in time") - case err := <-shutdownErr: - assert.Error(t, err, "Expected shutdown to return an error due to timeout") - assert.True(t, errors.Is(err, context.DeadlineExceeded), "Expected DeadlineExceeded error") - } + case <-time.After(2 * time.Second): + t.Fatal("shutdown did not complete in time") + case err := <-shutdownErr: + require.Error(t, err, "Expected shutdown to return an error due to timeout") + require.ErrorIs(t, err, context.DeadlineExceeded, "Expected DeadlineExceeded error") + } - assert.Equal(t, int32(1), atomic.LoadInt32(&shutdownHookCalled), "Shutdown hook was not called") + assert.Equal(t, int32(1), shutdownHookCalled.Load(), "Shutdown hook was not called") - select { - case err := <-serverErr: - assert.NoError(t, err, "Server should have shut down without error") - // default: - // Server is still running, which is expected as the long-running request prevented full shutdown - } + err := <-serverErr + assert.NoError(t, err, "Server should have shut down without error") + // default: + // Server is still running, which is expected as the long-running request prevented full shutdown } // go test -run Test_App_Mixed_Routes_WithSameLen diff --git a/go.mod b/go.mod index 2b5e60a1bf..dd7324b3f0 100644 --- a/go.mod +++ b/go.mod @@ -21,6 +21,7 @@ require ( github.com/philhofer/fwd v1.1.3-0.20240612014219-fbbf4953d986 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/valyala/tcplisten v1.0.0 // indirect + golang.org/dl v0.0.0-20241001165935-bedb0f791d00 // indirect golang.org/x/net v0.29.0 // indirect golang.org/x/sys v0.25.0 // indirect golang.org/x/text v0.18.0 // indirect diff --git a/go.sum b/go.sum index 42768451af..fd50cfe1c0 100644 --- a/go.sum +++ b/go.sum @@ -33,6 +33,8 @@ github.com/valyala/tcplisten v1.0.0 h1:rBHj/Xf+E1tRGZyWIWwJDiRY0zc1Js+CV5DqwacVS github.com/valyala/tcplisten v1.0.0/go.mod h1:T0xQ8SeCZGxckz9qRXTfG43PvQ/mcWh7FwZEA7Ioqkc= github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM= github.com/x448/float16 v0.8.4/go.mod h1:14CWIYCyZA/cWjXOioeEpHeN/83MdbZDRQHoFcYsOfg= +golang.org/dl v0.0.0-20241001165935-bedb0f791d00 h1:OX0WPBB1pQPZy1SL0+q5C/VuuM6e1wv6uEuB9iyBi/I= +golang.org/dl v0.0.0-20241001165935-bedb0f791d00/go.mod h1:fwQ+hlTD8I6TIzOGkQqxQNfE2xqR+y7SzGaDkksVFkw= golang.org/x/net v0.29.0 h1:5ORfpBpCs4HzDYoodCDBbwHzdR5UrLBZ3sOnUJmFoHo= golang.org/x/net v0.29.0/go.mod h1:gLkgy8jTGERgjzMic6DS9+SP0ajcu6Xu3Orq/SpETg0= golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= From b0bc70c71e15ca94bc2ae3c71903c0bf57d5e931 Mon Sep 17 00:00:00 2001 From: JIeJaitt <498938874@qq.com> Date: Mon, 28 Oct 2024 14:25:29 +0800 Subject: [PATCH 17/21] =?UTF-8?q?=F0=9F=8E=A8=20Style:=20add=20block=20in?= =?UTF-8?q?=20api.md?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- docs/api/fiber.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/api/fiber.md b/docs/api/fiber.md index 4cb1e0a77c..55566a109d 100644 --- a/docs/api/fiber.md +++ b/docs/api/fiber.md @@ -205,7 +205,7 @@ Shutdown gracefully shuts down the server without interrupting any active connec ShutdownWithTimeout will forcefully close any active connections after the timeout expires. -ShutdownWithContext shuts down the server including by force if the context's deadline is exceeded.Shutdown hooks will still be executed, even if an error occurs during the shutdown process, as they are deferred to ensure cleanup happens regardless of errors. +ShutdownWithContext shuts down the server including by force if the context's deadline is exceeded. Shutdown hooks will still be executed, even if an error occurs during the shutdown process, as they are deferred to ensure cleanup happens regardless of errors. ```go func (app *App) Shutdown() error From 44cbc627eb910e609a24978e6855892abab39773 Mon Sep 17 00:00:00 2001 From: JIeJaitt <498938874@qq.com> Date: Mon, 28 Oct 2024 16:37:52 +0800 Subject: [PATCH 18/21] =?UTF-8?q?=F0=9F=A9=B9=20Fix:=20go=20mod=20tidy?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- go.mod | 1 - go.sum | 2 -- 2 files changed, 3 deletions(-) diff --git a/go.mod b/go.mod index dd7324b3f0..2b5e60a1bf 100644 --- a/go.mod +++ b/go.mod @@ -21,7 +21,6 @@ require ( github.com/philhofer/fwd v1.1.3-0.20240612014219-fbbf4953d986 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect github.com/valyala/tcplisten v1.0.0 // indirect - golang.org/dl v0.0.0-20241001165935-bedb0f791d00 // indirect golang.org/x/net v0.29.0 // indirect golang.org/x/sys v0.25.0 // indirect golang.org/x/text v0.18.0 // indirect diff --git a/go.sum b/go.sum index fd50cfe1c0..42768451af 100644 --- a/go.sum +++ b/go.sum @@ -33,8 +33,6 @@ github.com/valyala/tcplisten v1.0.0 h1:rBHj/Xf+E1tRGZyWIWwJDiRY0zc1Js+CV5DqwacVS github.com/valyala/tcplisten v1.0.0/go.mod h1:T0xQ8SeCZGxckz9qRXTfG43PvQ/mcWh7FwZEA7Ioqkc= github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM= github.com/x448/float16 v0.8.4/go.mod h1:14CWIYCyZA/cWjXOioeEpHeN/83MdbZDRQHoFcYsOfg= -golang.org/dl v0.0.0-20241001165935-bedb0f791d00 h1:OX0WPBB1pQPZy1SL0+q5C/VuuM6e1wv6uEuB9iyBi/I= -golang.org/dl v0.0.0-20241001165935-bedb0f791d00/go.mod h1:fwQ+hlTD8I6TIzOGkQqxQNfE2xqR+y7SzGaDkksVFkw= golang.org/x/net v0.29.0 h1:5ORfpBpCs4HzDYoodCDBbwHzdR5UrLBZ3sOnUJmFoHo= golang.org/x/net v0.29.0/go.mod h1:gLkgy8jTGERgjzMic6DS9+SP0ajcu6Xu3Orq/SpETg0= golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= From 87b2aab9dd7c4253e624f58bdcb4cb3de7c4897d Mon Sep 17 00:00:00 2001 From: JIeJaitt <498938874@qq.com> Date: Tue, 10 Dec 2024 21:00:31 +0800 Subject: [PATCH 19/21] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Refactor:=20replaced?= =?UTF-8?q?=20OnShutdown=20=20by=20OnPreShutdown=20and=20OnPostShutdown?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app.go | 5 ++-- app_test.go | 9 +++---- hooks.go | 65 ++++++++++++++++++++++++++++++++++++++++----------- hooks_test.go | 11 +++++---- 4 files changed, 65 insertions(+), 25 deletions(-) diff --git a/app.go b/app.go index 2775e3f92d..82ae9d55b0 100644 --- a/app.go +++ b/app.go @@ -907,10 +907,11 @@ func (app *App) ShutdownWithContext(ctx context.Context) error { return ErrNotRunning } - // Execute shutdown hooks in a deferred function + // Execute the Shutdown hook if app.hooks != nil { - defer app.hooks.executeOnShutdownHooks() + app.hooks.executeOnPreShutdownHooks() } + defer app.hooks.executeOnPostShutdownHooks(nil) return app.server.ShutdownWithContext(ctx) } diff --git a/app_test.go b/app_test.go index 3becbf7e73..78f3825247 100644 --- a/app_test.go +++ b/app_test.go @@ -863,10 +863,11 @@ func Test_App_ShutdownWithContext(t *testing.T) { app := New() var shutdownHookCalled atomic.Int32 - app.Hooks().OnShutdown(func() error { - shutdownHookCalled.Store(1) - return nil - }) + // TODO: add test + // app.Hooks().OnShutdown(func() error { + // shutdownHookCalled.Store(1) + // return nil + // }) app.Get("/", func(ctx Ctx) error { time.Sleep(5 * time.Second) diff --git a/hooks.go b/hooks.go index 3da5c671ff..7f926832c3 100644 --- a/hooks.go +++ b/hooks.go @@ -11,9 +11,11 @@ type ( OnGroupHandler = func(Group) error OnGroupNameHandler = OnGroupHandler OnListenHandler = func(ListenData) error - OnShutdownHandler = func() error - OnForkHandler = func(int) error - OnMountHandler = func(*App) error + // OnShutdownHandler = func() error + OnPreShutdownHandler = func() error + OnPostShutdownHandler = func(error) error + OnForkHandler = func(int) error + OnMountHandler = func(*App) error ) // Hooks is a struct to use it with App. @@ -27,9 +29,11 @@ type Hooks struct { onGroup []OnGroupHandler onGroupName []OnGroupNameHandler onListen []OnListenHandler - onShutdown []OnShutdownHandler - onFork []OnForkHandler - onMount []OnMountHandler + // onShutdown []OnShutdownHandler + onPreShutdown []OnPreShutdownHandler + onPostShutdown []OnPostShutdownHandler + onFork []OnForkHandler + onMount []OnMountHandler } // ListenData is a struct to use it with OnListenHandler @@ -47,9 +51,11 @@ func newHooks(app *App) *Hooks { onGroupName: make([]OnGroupNameHandler, 0), onName: make([]OnNameHandler, 0), onListen: make([]OnListenHandler, 0), - onShutdown: make([]OnShutdownHandler, 0), - onFork: make([]OnForkHandler, 0), - onMount: make([]OnMountHandler, 0), + // onShutdown: make([]OnShutdownHandler, 0), + onPreShutdown: make([]OnPreShutdownHandler, 0), + onPostShutdown: make([]OnPostShutdownHandler, 0), + onFork: make([]OnForkHandler, 0), + onMount: make([]OnMountHandler, 0), } } @@ -96,10 +102,25 @@ func (h *Hooks) OnListen(handler ...OnListenHandler) { h.app.mutex.Unlock() } +// TODO：To be deleted, replaced by OnPreShutdown and OnPostShutdown // OnShutdown is a hook to execute user functions after Shutdown. -func (h *Hooks) OnShutdown(handler ...OnShutdownHandler) { +// func (h *Hooks) OnShutdown(handler ...OnShutdownHandler) { +// h.app.mutex.Lock() +// h.onShutdown = append(h.onShutdown, handler...) +// h.app.mutex.Unlock() +// } + +// OnPreShutdown is a hook to execute user functions before Shutdown. +func (h *Hooks) OnPreShutdown(handler ...OnPreShutdownHandler) { h.app.mutex.Lock() - h.onShutdown = append(h.onShutdown, handler...) + h.onPreShutdown = append(h.onPreShutdown, handler...) + h.app.mutex.Unlock() +} + +// OnPostShutdown is a hook to execute user functions after Shutdown. +func (h *Hooks) OnPostShutdown(handler ...OnPostShutdownHandler) { + h.app.mutex.Lock() + h.onPostShutdown = append(h.onPostShutdown, handler...) h.app.mutex.Unlock() } @@ -191,10 +212,26 @@ func (h *Hooks) executeOnListenHooks(listenData ListenData) error { return nil } -func (h *Hooks) executeOnShutdownHooks() { - for _, v := range h.onShutdown { +// func (h *Hooks) executeOnShutdownHooks() { +// for _, v := range h.onShutdown { +// if err := v(); err != nil { +// log.Errorf("failed to call shutdown hook: %v", err) +// } +// } +// } + +func (h *Hooks) executeOnPreShutdownHooks() { + for _, v := range h.onPreShutdown { if err := v(); err != nil { - log.Errorf("failed to call shutdown hook: %v", err) + log.Errorf("failed to call pre shutdown hook: %v", err) + } + } +} + +func (h *Hooks) executeOnPostShutdownHooks(err error) { + for _, v := range h.onPostShutdown { + if err := v(err); err != nil { + log.Errorf("failed to call pre shutdown hook: %v", err) } } } diff --git a/hooks_test.go b/hooks_test.go index f96f570706..c313031c08 100644 --- a/hooks_test.go +++ b/hooks_test.go @@ -188,12 +188,13 @@ func Test_Hook_OnShutdown(t *testing.T) { buf := bytebufferpool.Get() defer bytebufferpool.Put(buf) - app.Hooks().OnShutdown(func() error { - _, err := buf.WriteString("shutdowning") - require.NoError(t, err) + // TODO: add test + // app.Hooks().OnShutdown(func() error { + // _, err := buf.WriteString("shutdowning") + // require.NoError(t, err) - return nil - }) + // return nil + // }) require.NoError(t, app.Shutdown()) require.Equal(t, "shutdowning", buf.String()) From 33899134a450d26d7612e1fa35dc1770ab1096cf Mon Sep 17 00:00:00 2001 From: JIeJaitt <498938874@qq.com> Date: Wed, 11 Dec 2024 10:34:34 +0800 Subject: [PATCH 20/21] =?UTF-8?q?=E2=99=BB=EF=B8=8F=20Refactor:=20streamli?= =?UTF-8?q?ne=20post-shutdown=20hook=20execution=20in=20graceful=20shutdow?= =?UTF-8?q?n=20process?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app.go | 1 - hooks.go | 2 +- hooks_test.go | 32 +++++++++++++++++++++++++------- listen.go | 6 ++---- 4 files changed, 28 insertions(+), 13 deletions(-) diff --git a/app.go b/app.go index 82ae9d55b0..f6a24546a8 100644 --- a/app.go +++ b/app.go @@ -911,7 +911,6 @@ func (app *App) ShutdownWithContext(ctx context.Context) error { if app.hooks != nil { app.hooks.executeOnPreShutdownHooks() } - defer app.hooks.executeOnPostShutdownHooks(nil) return app.server.ShutdownWithContext(ctx) } diff --git a/hooks.go b/hooks.go index 7f926832c3..86a273d8fc 100644 --- a/hooks.go +++ b/hooks.go @@ -231,7 +231,7 @@ func (h *Hooks) executeOnPreShutdownHooks() { func (h *Hooks) executeOnPostShutdownHooks(err error) { for _, v := range h.onPostShutdown { if err := v(err); err != nil { - log.Errorf("failed to call pre shutdown hook: %v", err) + log.Errorf("failed to call post shutdown hook: %v", err) } } } diff --git a/hooks_test.go b/hooks_test.go index c313031c08..4cbfa20297 100644 --- a/hooks_test.go +++ b/hooks_test.go @@ -181,20 +181,38 @@ func Test_Hook_OnGroupName_Error(t *testing.T) { grp.Get("/test", testSimpleHandler) } -func Test_Hook_OnShutdown(t *testing.T) { +// func Test_Hook_OnShutdown(t *testing.T) { +// t.Parallel() +// app := New() + +// buf := bytebufferpool.Get() +// defer bytebufferpool.Put(buf) + +// // TODO: add test +// app.Hooks().OnShutdown(func() error { +// _, err := buf.WriteString("shutdowning") +// require.NoError(t, err) + +// return nil +// }) + +// require.NoError(t, app.Shutdown()) +// require.Equal(t, "shutdowning", buf.String()) +// } + +func Test_Hook_OnPrehutdown(t *testing.T) { t.Parallel() app := New() buf := bytebufferpool.Get() defer bytebufferpool.Put(buf) - // TODO: add test - // app.Hooks().OnShutdown(func() error { - // _, err := buf.WriteString("shutdowning") - // require.NoError(t, err) + app.Hooks().OnPreShutdown(func() error { + _, err := buf.WriteString("shutdowning") + require.NoError(t, err) - // return nil - // }) + return nil + }) require.NoError(t, app.Shutdown()) require.Equal(t, "shutdowning", buf.String()) diff --git a/listen.go b/listen.go index e0c5536968..739d81983a 100644 --- a/listen.go +++ b/listen.go @@ -502,11 +502,9 @@ func (app *App) gracefulShutdown(ctx context.Context, cfg ListenConfig) { } if err != nil { - cfg.OnShutdownError(err) + app.hooks.executeOnPostShutdownHooks(err) return } - if success := cfg.OnShutdownSuccess; success != nil { - success() - } + app.hooks.executeOnPostShutdownHooks(nil) } From 6aeb04825241e1dc41be1760cfe090815b042155 Mon Sep 17 00:00:00 2001 From: JIeJaitt <498938874@qq.com> Date: Wed, 11 Dec 2024 16:59:12 +0800 Subject: [PATCH 21/21] =?UTF-8?q?=F0=9F=9A=A8=20Test:=20add=20test=20for?= =?UTF-8?q?=20gracefulShutdown?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app_test.go | 10 +-- hooks.go | 51 ++++-------- hooks_test.go | 93 ++++++++++++++++----- listen.go | 20 ----- listen_test.go | 219 ++++++++++++++++--------------------------------- 5 files changed, 162 insertions(+), 231 deletions(-) diff --git a/app_test.go b/app_test.go index 78f3825247..cd919b21f6 100644 --- a/app_test.go +++ b/app_test.go @@ -863,11 +863,11 @@ func Test_App_ShutdownWithContext(t *testing.T) { app := New() var shutdownHookCalled atomic.Int32 - // TODO: add test - // app.Hooks().OnShutdown(func() error { - // shutdownHookCalled.Store(1) - // return nil - // }) + + app.Hooks().OnPreShutdown(func() error { + shutdownHookCalled.Store(1) + return nil + }) app.Get("/", func(ctx Ctx) error { time.Sleep(5 * time.Second) diff --git a/hooks.go b/hooks.go index 86a273d8fc..314717d04b 100644 --- a/hooks.go +++ b/hooks.go @@ -6,12 +6,11 @@ import ( // OnRouteHandler Handlers define a function to create hooks for Fiber. type ( - OnRouteHandler = func(Route) error - OnNameHandler = OnRouteHandler - OnGroupHandler = func(Group) error - OnGroupNameHandler = OnGroupHandler - OnListenHandler = func(ListenData) error - // OnShutdownHandler = func() error + OnRouteHandler = func(Route) error + OnNameHandler = OnRouteHandler + OnGroupHandler = func(Group) error + OnGroupNameHandler = OnGroupHandler + OnListenHandler = func(ListenData) error OnPreShutdownHandler = func() error OnPostShutdownHandler = func(error) error OnForkHandler = func(int) error @@ -24,12 +23,11 @@ type Hooks struct { app *App // Hooks - onRoute []OnRouteHandler - onName []OnNameHandler - onGroup []OnGroupHandler - onGroupName []OnGroupNameHandler - onListen []OnListenHandler - // onShutdown []OnShutdownHandler + onRoute []OnRouteHandler + onName []OnNameHandler + onGroup []OnGroupHandler + onGroupName []OnGroupNameHandler + onListen []OnListenHandler onPreShutdown []OnPreShutdownHandler onPostShutdown []OnPostShutdownHandler onFork []OnForkHandler @@ -45,13 +43,12 @@ type ListenData struct { func newHooks(app *App) *Hooks { return &Hooks{ - app: app, - onRoute: make([]OnRouteHandler, 0), - onGroup: make([]OnGroupHandler, 0), - onGroupName: make([]OnGroupNameHandler, 0), - onName: make([]OnNameHandler, 0), - onListen: make([]OnListenHandler, 0), - // onShutdown: make([]OnShutdownHandler, 0), + app: app, + onRoute: make([]OnRouteHandler, 0), + onGroup: make([]OnGroupHandler, 0), + onGroupName: make([]OnGroupNameHandler, 0), + onName: make([]OnNameHandler, 0), + onListen: make([]OnListenHandler, 0), onPreShutdown: make([]OnPreShutdownHandler, 0), onPostShutdown: make([]OnPostShutdownHandler, 0), onFork: make([]OnForkHandler, 0), @@ -102,14 +99,6 @@ func (h *Hooks) OnListen(handler ...OnListenHandler) { h.app.mutex.Unlock() } -// TODO：To be deleted, replaced by OnPreShutdown and OnPostShutdown -// OnShutdown is a hook to execute user functions after Shutdown. -// func (h *Hooks) OnShutdown(handler ...OnShutdownHandler) { -// h.app.mutex.Lock() -// h.onShutdown = append(h.onShutdown, handler...) -// h.app.mutex.Unlock() -// } - // OnPreShutdown is a hook to execute user functions before Shutdown. func (h *Hooks) OnPreShutdown(handler ...OnPreShutdownHandler) { h.app.mutex.Lock() @@ -212,14 +201,6 @@ func (h *Hooks) executeOnListenHooks(listenData ListenData) error { return nil } -// func (h *Hooks) executeOnShutdownHooks() { -// for _, v := range h.onShutdown { -// if err := v(); err != nil { -// log.Errorf("failed to call shutdown hook: %v", err) -// } -// } -// } - func (h *Hooks) executeOnPreShutdownHooks() { for _, v := range h.onPreShutdown { if err := v(); err != nil { diff --git a/hooks_test.go b/hooks_test.go index 4cbfa20297..b39146b15e 100644 --- a/hooks_test.go +++ b/hooks_test.go @@ -181,25 +181,6 @@ func Test_Hook_OnGroupName_Error(t *testing.T) { grp.Get("/test", testSimpleHandler) } -// func Test_Hook_OnShutdown(t *testing.T) { -// t.Parallel() -// app := New() - -// buf := bytebufferpool.Get() -// defer bytebufferpool.Put(buf) - -// // TODO: add test -// app.Hooks().OnShutdown(func() error { -// _, err := buf.WriteString("shutdowning") -// require.NoError(t, err) - -// return nil -// }) - -// require.NoError(t, app.Shutdown()) -// require.Equal(t, "shutdowning", buf.String()) -// } - func Test_Hook_OnPrehutdown(t *testing.T) { t.Parallel() app := New() @@ -208,14 +189,84 @@ func Test_Hook_OnPrehutdown(t *testing.T) { defer bytebufferpool.Put(buf) app.Hooks().OnPreShutdown(func() error { - _, err := buf.WriteString("shutdowning") + _, err := buf.WriteString("pre-shutdowning") require.NoError(t, err) return nil }) require.NoError(t, app.Shutdown()) - require.Equal(t, "shutdowning", buf.String()) + require.Equal(t, "pre-shutdowning", buf.String()) +} + +func Test_Hook_OnPostShutdown(t *testing.T) { + t.Run("should execute post shutdown hook with error", func(t *testing.T) { + app := New() + + hookCalled := false + var receivedErr error + expectedErr := errors.New("test shutdown error") + + app.Hooks().OnPostShutdown(func(err error) error { + hookCalled = true + receivedErr = err + return nil + }) + + go func() { + _ = app.Listen(":0") + }() + + time.Sleep(100 * time.Millisecond) + + app.hooks.executeOnPostShutdownHooks(expectedErr) + + if !hookCalled { + t.Fatal("hook was not called") + } + + if receivedErr != expectedErr { + t.Fatalf("hook received wrong error: want %v, got %v", expectedErr, receivedErr) + } + }) + + t.Run("should execute multiple hooks in order", func(t *testing.T) { + app := New() + + execution := make([]int, 0) + + app.Hooks().OnPostShutdown(func(err error) error { + execution = append(execution, 1) + return nil + }) + + app.Hooks().OnPostShutdown(func(err error) error { + execution = append(execution, 2) + return nil + }) + + app.hooks.executeOnPostShutdownHooks(nil) + + if len(execution) != 2 { + t.Fatalf("expected 2 hooks to execute, got %d", len(execution)) + } + + if execution[0] != 1 || execution[1] != 2 { + t.Fatal("hooks executed in wrong order") + } + }) + + t.Run("should handle hook error", func(t *testing.T) { + app := New() + hookErr := errors.New("hook error") + + app.Hooks().OnPostShutdown(func(err error) error { + return hookErr + }) + + // Should not panic + app.hooks.executeOnPostShutdownHooks(nil) + }) } func Test_Hook_OnListen(t *testing.T) { diff --git a/listen.go b/listen.go index 739d81983a..8fdb0ff453 100644 --- a/listen.go +++ b/listen.go @@ -60,17 +60,6 @@ type ListenConfig struct { // Default: nil BeforeServeFunc func(app *App) error `json:"before_serve_func"` - // OnShutdownError allows to customize error behavior when to graceful shutdown server by given signal. - // - // Print error with log.Fatalf() by default. - // Default: nil - OnShutdownError func(err error) - - // OnShutdownSuccess allows to customize success behavior when to graceful shutdown server by given signal. - // - // Default: nil - OnShutdownSuccess func() - // AutoCertManager manages TLS certificates automatically using the ACME protocol, // Enables integration with Let's Encrypt or other ACME-compatible providers. // @@ -129,9 +118,6 @@ func listenConfigDefault(config ...ListenConfig) ListenConfig { if len(config) < 1 { return ListenConfig{ ListenerNetwork: NetworkTCP4, - OnShutdownError: func(err error) { - log.Fatalf("shutdown: %v", err) //nolint:revive // It's an option - }, ShutdownTimeout: 10 * time.Second, } } @@ -141,12 +127,6 @@ func listenConfigDefault(config ...ListenConfig) ListenConfig { cfg.ListenerNetwork = NetworkTCP4 } - if cfg.OnShutdownError == nil { - cfg.OnShutdownError = func(err error) { - log.Fatalf("shutdown: %v", err) //nolint:revive // It's an option - } - } - return cfg } diff --git a/listen_test.go b/listen_test.go index 123cf2b3b8..f1376a6e7c 100644 --- a/listen_test.go +++ b/listen_test.go @@ -15,6 +15,7 @@ import ( "testing" "time" + "github.com/gofiber/utils/v2" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/valyala/fasthttp" @@ -37,193 +38,111 @@ func Test_Listen(t *testing.T) { // go test -run Test_Listen_Graceful_Shutdown func Test_Listen_Graceful_Shutdown(t *testing.T) { - var mu sync.Mutex - var shutdown bool - - app := New() - - app.Get("/", func(c Ctx) error { - return c.SendString(c.Hostname()) + t.Run("Basic Graceful Shutdown", func(t *testing.T) { + testGracefulShutdown(t, 0) }) - ln := fasthttputil.NewInmemoryListener() - errs := make(chan error) - - go func() { - ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) - defer cancel() - - errs <- app.Listener(ln, ListenConfig{ - DisableStartupMessage: true, - GracefulContext: ctx, - OnShutdownSuccess: func() { - mu.Lock() - shutdown = true - mu.Unlock() - }, - }) - }() - - // Server readiness check - for i := 0; i < 10; i++ { - conn, err := ln.Dial() - if err == nil { - conn.Close() //nolint:errcheck // ignore error - break - } - // Wait a bit before retrying - time.Sleep(100 * time.Millisecond) - if i == 9 { - t.Fatalf("Server did not become ready in time: %v", err) - } - } - - testCases := []struct { - ExpectedErr error - ExpectedBody string - Time time.Duration - ExpectedStatusCode int - }{ - {Time: 500 * time.Millisecond, ExpectedBody: "example.com", ExpectedStatusCode: StatusOK, ExpectedErr: nil}, - {Time: 3 * time.Second, ExpectedBody: "", ExpectedStatusCode: StatusOK, ExpectedErr: fasthttputil.ErrInmemoryListenerClosed}, - } - - for _, tc := range testCases { - time.Sleep(tc.Time) - - req := fasthttp.AcquireRequest() - req.SetRequestURI("http://example.com") - - client := fasthttp.HostClient{} - client.Dial = func(_ string) (net.Conn, error) { return ln.Dial() } - - resp := fasthttp.AcquireResponse() - err := client.Do(req, resp) - - require.Equal(t, tc.ExpectedErr, err) - require.Equal(t, tc.ExpectedStatusCode, resp.StatusCode()) - require.Equal(t, tc.ExpectedBody, string(resp.Body())) - - fasthttp.ReleaseRequest(req) - fasthttp.ReleaseResponse(resp) - } - - mu.Lock() - err := <-errs - require.True(t, shutdown) - require.NoError(t, err) - mu.Unlock() + t.Run("Shutdown With Timeout", func(t *testing.T) { + testGracefulShutdown(t, 500*time.Millisecond) + }) } -// go test -run Test_Listen_Graceful_Shutdown_Timeout -func Test_Listen_Graceful_Shutdown_Timeout(t *testing.T) { +func testGracefulShutdown(t *testing.T, shutdownTimeout time.Duration) { var mu sync.Mutex - var shutdownSuccess bool - var shutdownTimeoutError error + var shutdown bool app := New() - app.Get("/", func(c Ctx) error { return c.SendString(c.Hostname()) }) ln := fasthttputil.NewInmemoryListener() - errs := make(chan error) + errs := make(chan error, 1) - go func() { - ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) - defer cancel() + app.hooks.OnPostShutdown(func(err error) error { + mu.Lock() + defer mu.Unlock() + shutdown = true + return nil + }) + + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second) + defer cancel() + go func() { errs <- app.Listener(ln, ListenConfig{ DisableStartupMessage: true, GracefulContext: ctx, - ShutdownTimeout: 500 * time.Millisecond, - OnShutdownSuccess: func() { - mu.Lock() - shutdownSuccess = true - mu.Unlock() - }, - OnShutdownError: func(err error) { - mu.Lock() - shutdownTimeoutError = err - mu.Unlock() - }, + ShutdownTimeout: shutdownTimeout, }) }() - // Server readiness check - for i := 0; i < 10; i++ { + require.Eventually(t, func() bool { conn, err := ln.Dial() - // To test a graceful shutdown timeout, do not close the connection. if err == nil { - _ = conn - break - } - // Wait a bit before retrying - time.Sleep(100 * time.Millisecond) - if i == 9 { - t.Fatalf("Server did not become ready in time: %v", err) + conn.Close() + return true } + return false + }, time.Second, 100*time.Millisecond, "Server failed to become ready") + + client := fasthttp.HostClient{ + Dial: func(_ string) (net.Conn, error) { return ln.Dial() }, } testCases := []struct { - ExpectedErr error - ExpectedShutdownError error - ExpectedBody string - Time time.Duration - ExpectedStatusCode int - ExpectedShutdownSuccess bool + name string + waitTime time.Duration + expectedBody string + expectedStatusCode int + expectedErr error + closeConnection bool }{ { - Time: 100 * time.Millisecond, - ExpectedBody: "example.com", - ExpectedStatusCode: StatusOK, - ExpectedErr: nil, - ExpectedShutdownError: nil, - ExpectedShutdownSuccess: false, + name: "Server running normally", + waitTime: 500 * time.Millisecond, + expectedBody: "example.com", + expectedStatusCode: StatusOK, + expectedErr: nil, + closeConnection: true, }, { - Time: 3 * time.Second, - ExpectedBody: "", - ExpectedStatusCode: StatusOK, - ExpectedErr: fasthttputil.ErrInmemoryListenerClosed, - ExpectedShutdownError: context.DeadlineExceeded, - ExpectedShutdownSuccess: false, + name: "Server shutdown complete", + waitTime: 3 * time.Second, + expectedBody: "", + expectedStatusCode: StatusOK, + expectedErr: fasthttputil.ErrInmemoryListenerClosed, + closeConnection: true, }, } for _, tc := range testCases { - time.Sleep(tc.Time) - - req := fasthttp.AcquireRequest() - req.SetRequestURI("http://example.com") - - client := fasthttp.HostClient{} - client.Dial = func(_ string) (net.Conn, error) { return ln.Dial() } - - resp := fasthttp.AcquireResponse() - err := client.Do(req, resp) - - if err == nil { - require.NoError(t, err) - require.Equal(t, tc.ExpectedStatusCode, resp.StatusCode()) - require.Equal(t, tc.ExpectedBody, string(resp.Body())) - } else { - require.ErrorIs(t, err, tc.ExpectedErr) - } - - mu.Lock() - require.Equal(t, tc.ExpectedShutdownSuccess, shutdownSuccess) - require.Equal(t, tc.ExpectedShutdownError, shutdownTimeoutError) - mu.Unlock() - - fasthttp.ReleaseRequest(req) - fasthttp.ReleaseResponse(resp) + tc := tc + t.Run(tc.name, func(t *testing.T) { + time.Sleep(tc.waitTime) + + req := fasthttp.AcquireRequest() + defer fasthttp.ReleaseRequest(req) + req.SetRequestURI("http://example.com") + + resp := fasthttp.AcquireResponse() + defer fasthttp.ReleaseResponse(resp) + + err := client.Do(req, resp) + + if tc.expectedErr == nil { + assert.NoError(t, err) + assert.Equal(t, tc.expectedStatusCode, resp.StatusCode()) + assert.Equal(t, tc.expectedBody, utils.UnsafeString(resp.Body())) + } else { + assert.ErrorIs(t, err, tc.expectedErr) + } + }) } mu.Lock() - err := <-errs - require.NoError(t, err) + assert.True(t, shutdown) + assert.NoError(t, <-errs) mu.Unlock() }