From f7395d560f0be118394a3248d6d21e788722b603 Mon Sep 17 00:00:00 2001 From: Vincent Lee Date: Wed, 8 Jul 2015 18:59:32 +0800 Subject: [PATCH 1/5] handle closed listeners --- gracenet/net.go | 43 ++++++++++++++++++++++++++++++++++--------- 1 file changed, 34 insertions(+), 9 deletions(-) diff --git a/gracenet/net.go b/gracenet/net.go index 268b524..f13497a 100644 --- a/gracenet/net.go +++ b/gracenet/net.go @@ -31,10 +31,11 @@ var originalWD, _ = os.Getwd() // Net provides the family of Listen functions and maintains the associated // state. Typically you will have only once instance of Net per application. type Net struct { - inherited []net.Listener - active []net.Listener - mutex sync.Mutex - inheritOnce sync.Once + inherited []net.Listener + active []net.Listener + closedListeners []net.Listener + mutex sync.Mutex + inheritOnce sync.Once // used in tests to override the default behavior of starting from fd 3. fdStart int @@ -168,6 +169,13 @@ func (n *Net) ListenUnix(nett string, laddr *net.UnixAddr) (*net.UnixListener, e return l, nil } +func (n *Net) Close(l net.Listener) { + n.mutex.Lock() + defer n.mutex.Unlock() + n.closedListeners = append(n.closedListeners, l) + l.Close() +} + // activeListeners returns a snapshot copy of the active listeners. func (n *Net) activeListeners() ([]net.Listener, error) { n.mutex.Lock() @@ -209,14 +217,31 @@ func (n *Net) StartProcess() (int, error) { return 0, err } + n.mutex.Lock() + closedList := make([]net.Addr, len(n.closedListeners)) + for i, l := range n.closedListeners { + closedList[i] = l.Addr() + } + n.mutex.Unlock() // Extract the fds from the listeners. - files := make([]*os.File, len(listeners)) - for i, l := range listeners { - files[i], err = l.(filer).File() + files := make([]*os.File, 0, len(listeners)-len(closedList)) + for _, l := range listeners { + is_closed := false + for _, closedAddr := range closedList { + if isSameAddr(l.Addr(), closedAddr) { + is_closed = true + break + } + } + if is_closed { + continue + } + f, err := l.(filer).File() if err != nil { return 0, err } - defer files[i].Close() + files = append(files, f) + defer f.Close() } // Use the original binary location. This works with symlinks such that if @@ -233,7 +258,7 @@ func (n *Net) StartProcess() (int, error) { env = append(env, v) } } - env = append(env, fmt.Sprintf("%s%d", envCountKeyPrefix, len(listeners))) + env = append(env, fmt.Sprintf("%s%d", envCountKeyPrefix, len(files))) allFiles := append([]*os.File{os.Stdin, os.Stdout, os.Stderr}, files...) process, err := os.StartProcess(argv0, os.Args, &os.ProcAttr{ From 732c685a04207532881d93f07c40251db7543b31 Mon Sep 17 00:00:00 2001 From: Vincent Lee Date: Tue, 11 Aug 2015 19:43:34 +0800 Subject: [PATCH 2/5] go like nameing --- gracenet/net.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/gracenet/net.go b/gracenet/net.go index f13497a..ff81d73 100644 --- a/gracenet/net.go +++ b/gracenet/net.go @@ -226,14 +226,14 @@ func (n *Net) StartProcess() (int, error) { // Extract the fds from the listeners. files := make([]*os.File, 0, len(listeners)-len(closedList)) for _, l := range listeners { - is_closed := false + isClosed := false for _, closedAddr := range closedList { if isSameAddr(l.Addr(), closedAddr) { - is_closed = true + isClosed = true break } } - if is_closed { + if isClosed { continue } f, err := l.(filer).File() From 6fa76ff4aded032b349a56dfe2bbe3beb33460e1 Mon Sep 17 00:00:00 2001 From: Vincent Lee Date: Thu, 13 Aug 2015 16:04:37 +0800 Subject: [PATCH 3/5] add test case for close listener --- gracenet/net.go | 36 ++++++++++++++-------- gracenet/net_test.go | 72 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 95 insertions(+), 13 deletions(-) diff --git a/gracenet/net.go b/gracenet/net.go index ff81d73..10283ae 100644 --- a/gracenet/net.go +++ b/gracenet/net.go @@ -206,17 +206,7 @@ func isSameAddr(a1, a2 net.Addr) bool { return a1s == a2s } -// StartProcess starts a new process passing it the active listeners. It -// doesn't fork, but starts a new process using the same environment and -// arguments as when it was originally started. This allows for a newly -// deployed binary to be started. It returns the pid of the newly started -// process when successful. -func (n *Net) StartProcess() (int, error) { - listeners, err := n.activeListeners() - if err != nil { - return 0, err - } - +func (n *Net) prepareInheriteFiles(listeners []net.Listener) ([]*os.File, error) { n.mutex.Lock() closedList := make([]net.Addr, len(n.closedListeners)) for i, l := range n.closedListeners { @@ -238,12 +228,32 @@ func (n *Net) StartProcess() (int, error) { } f, err := l.(filer).File() if err != nil { - return 0, err + return nil, err } files = append(files, f) - defer f.Close() } + return files, nil +} +// StartProcess starts a new process passing it the active listeners. It +// doesn't fork, but starts a new process using the same environment and +// arguments as when it was originally started. This allows for a newly +// deployed binary to be started. It returns the pid of the newly started +// process when successful. +func (n *Net) StartProcess() (int, error) { + listeners, err := n.activeListeners() + if err != nil { + return 0, err + } + + // Extract the fds from the listeners. + files, err := n.prepareInheriteFiles(listeners) + if err != nil { + return 0, err + } + for _, f := range files { + defer f.Close() + } // Use the original binary location. This works with symlinks such that if // the file it points to has been changed we will use the updated symlink. argv0, err := exec.LookPath(os.Args[0]) diff --git a/gracenet/net_test.go b/gracenet/net_test.go index 8b79476..623888a 100644 --- a/gracenet/net_test.go +++ b/gracenet/net_test.go @@ -349,6 +349,78 @@ func TestPortZeroTwice(t *testing.T) { ensure.Nil(t, l2.Close()) } +func TestPrepareInheriteFiles(t *testing.T) { + var n Net + + port1, err := freeport.Get() + ensure.Nil(t, err) + addr1 := fmt.Sprintf(":%d", port1) + l1, err := net.Listen("tcp", addr1) + ensure.Nil(t, err) + + port2, err := freeport.Get() + ensure.Nil(t, err) + addr2 := fmt.Sprintf(":%d", port2) + l2, err := net.Listen("tcp", addr2) + ensure.Nil(t, err) + + files, err := n.prepareInheriteFiles([]net.Listener{l1, l2}) + ensure.Nil(t, err) + ensure.DeepEqual(t, len(files), 2) + + // assign both to prevent GC from kicking in the finalizer + fds := []int{dup(t, int(files[0].Fd())), dup(t, int(files[0].Fd()))} + n.fdStart = fds[0] + os.Setenv(envCountKey, "2") + // Close these after to ensure we get coalaced file descriptors. + ensure.Nil(t, l1.Close()) + ensure.Nil(t, l2.Close()) + + ensure.Nil(t, n.inherit()) + ensure.DeepEqual(t, len(n.inherited), 2) +} + +func TestCloseListener(t *testing.T) { + var n Net + + port1, err := freeport.Get() + ensure.Nil(t, err) + addr1 := fmt.Sprintf(":%d", port1) + l1, err := net.Listen("tcp", addr1) + ensure.Nil(t, err) + + port2, err := freeport.Get() + ensure.Nil(t, err) + addr2 := fmt.Sprintf(":%d", port2) + l2, err := net.Listen("tcp", addr2) + ensure.Nil(t, err) + + // close using the gracenet interface + n.Close(l2) + + files, err := n.prepareInheriteFiles([]net.Listener{l1, l2}) + ensure.Nil(t, err) + ensure.DeepEqual(t, len(files), 1) + + // assign both to prevent GC from kicking in the finalizer + fds := []int{dup(t, int(files[0].Fd()))} + n.fdStart = fds[0] + os.Setenv(envCountKey, "1") + // Close these after to ensure we get coalaced file descriptors. + ensure.Nil(t, l1.Close()) + + ensure.Nil(t, n.inherit()) + ensure.DeepEqual(t, len(n.inherited), 1) + + l1, err = n.Listen("tcp", addr1) + + ensure.Nil(t, err) + ensure.DeepEqual(t, len(n.active), 1) + ensure.DeepEqual(t, n.inherited[0], nil) + ensure.Nil(t, l1.Close()) + ensure.Nil(t, files[0].Close()) +} + // We dup file descriptors because the os.Files are closed by a finalizer when // they are GCed, which interacts badly with the fact that the OS reuses fds, // and that we emulating inheriting the fd by it's integer value in our tests. From 25f523f775e8823de526678f9105f6b6fe19d904 Mon Sep 17 00:00:00 2001 From: Vincent Lee Date: Mon, 4 Jan 2016 10:35:31 +0800 Subject: [PATCH 4/5] change import --- gracedemo/demo.go | 2 +- gracehttp/http.go | 2 +- gracehttp/testbin_test.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/gracedemo/demo.go b/gracedemo/demo.go index 842d880..759a922 100644 --- a/gracedemo/demo.go +++ b/gracedemo/demo.go @@ -9,7 +9,7 @@ import ( "os" "time" - "github.com/facebookgo/grace/gracehttp" + "github.com/absolute8511/grace/gracehttp" ) var ( diff --git a/gracehttp/http.go b/gracehttp/http.go index 93e26a6..cf88f77 100644 --- a/gracehttp/http.go +++ b/gracehttp/http.go @@ -15,7 +15,7 @@ import ( "sync" "syscall" - "github.com/facebookgo/grace/gracenet" + "github.com/absolute8511/grace/gracenet" "github.com/facebookgo/httpdown" ) diff --git a/gracehttp/testbin_test.go b/gracehttp/testbin_test.go index b92ec2d..3d2fa09 100644 --- a/gracehttp/testbin_test.go +++ b/gracehttp/testbin_test.go @@ -13,7 +13,7 @@ import ( "testing" "time" - "github.com/facebookgo/grace/gracehttp" + "github.com/absolute8511/grace/gracehttp" ) func TestMain(m *testing.M) { From 3991ea4c07db8ef7254d46391d79dbf7308e5204 Mon Sep 17 00:00:00 2001 From: wangjian-pg Date: Fri, 22 Dec 2017 18:42:18 +0800 Subject: [PATCH 5/5] http: support 'ServeWithNet' --- gracehttp/http.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/gracehttp/http.go b/gracehttp/http.go index f781959..de0803b 100644 --- a/gracehttp/http.go +++ b/gracehttp/http.go @@ -186,6 +186,12 @@ func ServeWithOptions(servers []*http.Server, options ...option) error { return a.run() } +func ServeWithNet(servers []*http.Server, grace *gracenet.Net) error { + a := newApp(servers) + a.net = grace + return a.run() +} + // Serve will serve the given http.Servers and will monitor for signals // allowing for graceful termination (SIGTERM) or restart (SIGUSR2). func Serve(servers ...*http.Server) error {