Skip to content

Commit 12a0300

Browse files
committed
netutil: ListenUnix...() to leave responsibility of listener closing to outside
this will prevent double close (and associated error messages coming from HTTP server shutdown etc.)
1 parent a880178 commit 12a0300

File tree

2 files changed

+19
-34
lines changed

2 files changed

+19
-34
lines changed

net/http/httputils/utils.go

+3-3
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@ func NoCacheHeaders(w http.ResponseWriter) {
2424
w.Header().Set("Cache-Control", "no-store, must-revalidate")
2525
}
2626

27-
// helper for adapting context cancellation to shutdown the HTTP listener
28-
func CancelableServer(ctx context.Context, srv *http.Server, listener func() error) error {
27+
// helper for adapting context cancellation to shutdown the HTTP server
28+
func CancelableServer(ctx context.Context, srv *http.Server, serve func() error) error {
2929
shutdownerCtx, cancel := context.WithCancel(ctx)
3030

3131
shutdownResult := make(chan error, 1)
@@ -40,7 +40,7 @@ func CancelableServer(ctx context.Context, srv *http.Server, listener func() err
4040
shutdownResult <- srv.Shutdown(context.Background())
4141
}()
4242

43-
err := listener()
43+
err := serve()
4444

4545
// ask shutdowner to stop. this is useful only for cleanup where listener failed before
4646
// it was requested to shut down b/c parent cancellation didn't happen and thus the

net/netutil/unixsocketutil.go

+16-31
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
package netutil
33

44
import (
5-
"context"
65
"fmt"
76
"net"
87
"os"
@@ -19,63 +18,49 @@ var (
1918
allowEveryone = osutil.FileMode(osutil.OwnerRW, osutil.GroupRW, osutil.OtherRW)
2019
)
2120

22-
func ListenUnixAllowOwner(ctx context.Context, sockPath string, with func(net.Listener) error) error {
23-
return ListenUnixWithMode(ctx, sockPath, &allowOwner, with)
21+
func ListenUnixAllowOwner(sockPath string, with func(net.Listener) error) error {
22+
return ListenUnixWithMode(sockPath, &allowOwner, with)
2423
}
2524

26-
func ListenUnixAllowOwnerAndGroup(ctx context.Context, sockPath string, with func(net.Listener) error) error {
27-
return ListenUnixWithMode(ctx, sockPath, &allowOwnerAndGroup, with)
25+
func ListenUnixAllowOwnerAndGroup(sockPath string, with func(net.Listener) error) error {
26+
return ListenUnixWithMode(sockPath, &allowOwnerAndGroup, with)
2827
}
2928

30-
func ListenUnixAllowEveryone(ctx context.Context, sockPath string, with func(net.Listener) error) error {
31-
return ListenUnixWithMode(ctx, sockPath, &allowEveryone, with)
29+
func ListenUnixAllowEveryone(sockPath string, with func(net.Listener) error) error {
30+
return ListenUnixWithMode(sockPath, &allowEveryone, with)
3231
}
3332

3433
// pass nil os.FileMode if you don't want to Chmod() the socket file
35-
func ListenUnixWithMode(
36-
ctx context.Context,
37-
sockPath string,
38-
mode *os.FileMode,
39-
with func(net.Listener) error,
40-
) error {
34+
//
35+
// NOTE: `with()` is responsible for closing the obtained listener. for example the HTTP server
36+
// launched from `with()` must be able to do graceful shutdown and `http.Server.Serve()` always closes the listener.
37+
func ListenUnixWithMode(sockPath string, mode *os.FileMode, with func(net.Listener) error) error {
38+
withErr := func(err error) error { return fmt.Errorf("ListenUnix: %w", err) }
39+
4140
exists, err := osutil.Exists(sockPath)
4241
if err != nil {
43-
return fmt.Errorf("ListenUnix: exists: %w", err)
42+
return withErr(fmt.Errorf("exists: %w", err))
4443
}
4544

4645
// clean-up old socket
4746
if exists {
4847
if err := os.Remove(sockPath); err != nil {
49-
return fmt.Errorf("ListenUnix: cleanup previous: %w", err)
48+
return withErr(fmt.Errorf("cleanup previous socket: %w", err))
5049
}
5150
}
5251

5352
listener, err := net.Listen("unix", sockPath)
5453
if err != nil {
55-
return fmt.Errorf("ListenUnix: %w", err)
54+
return withErr(err)
5655
}
5756

5857
defer func() { // socket file was created. cleanup, so hopefully the next user doesn't need os.Remove()
5958
_ = os.Remove(sockPath)
6059
}()
6160

62-
listenerCtx, cancel := context.WithCancel(ctx) // explained in next block
63-
defer cancel()
64-
65-
// stop listener when:
66-
//
67-
// a) parent ctx done
68-
// b) we're exiting due to Chmod() failing
69-
// c) with() exits due to listener not closed but Accept() failing (kinda grey area - can that happen?)
70-
// ^ this signalled by defer cancel()
71-
go func() {
72-
<-listenerCtx.Done()
73-
listener.Close()
74-
}()
75-
7661
if mode != nil {
7762
if err := os.Chmod(sockPath, *mode); err != nil {
78-
return fmt.Errorf("ListenUnix: %w", err)
63+
return withErr(err)
7964
}
8065
}
8166

0 commit comments

Comments
 (0)