Skip to content

Commit

Permalink
Remove windows workarounds for statting sockets
Browse files Browse the repository at this point in the history
See: golang/go#33357

Go 1.22 has made it so that os.Stat now follows all reparse points, of which IO_REPARSE_TAG_AF_UNIX (the reparse point that os.ModeSocket should have been triggering) is one
  • Loading branch information
moskyb committed Feb 12, 2024
1 parent 3ba4552 commit d4bbaf8
Show file tree
Hide file tree
Showing 4 changed files with 14 additions and 35 deletions.
18 changes: 6 additions & 12 deletions internal/socket/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"net"
"net/http"
"os"
"runtime"
)

// Error response is the response body for any errors that occur
Expand Down Expand Up @@ -37,17 +36,12 @@ type Client struct {
// NewClient creates a new Client. The context is used for an internal check
// that the socket can be dialled.
func NewClient(ctx context.Context, path, token string) (*Client, error) {
// Check the socket path exists and is a socket.
// Note that os.ModeSocket might not be set on Windows.
// (https://github.com/golang/go/issues/33357)
if runtime.GOOS != "windows" {
fi, err := os.Stat(path)
if err != nil {
return nil, fmt.Errorf("stat socket: %w", err)
}
if fi.Mode()&os.ModeSocket == 0 {
return nil, fmt.Errorf("%q is not a socket", path)
}
fi, err := os.Stat(path)
if err != nil {
return nil, fmt.Errorf("stat socket: %w", err)
}
if fi.Mode()&os.ModeSocket == 0 {
return nil, fmt.Errorf("%q is not a socket", path)
}

dialer := net.Dialer{}
Expand Down
20 changes: 7 additions & 13 deletions internal/socket/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import (
"net/http"
"os"
"path/filepath"
"runtime"
"sync/atomic"
"testing"
"time"
Expand Down Expand Up @@ -52,18 +51,13 @@ func TestServerStartStop(t *testing.T) {
t.Fatalf("srv.Start() = %v", err)
}

// Check the socket path exists and is a socket.
// Note that os.ModeSocket might not be set on Windows.
// (https://github.com/golang/go/issues/33357)
if runtime.GOOS != "windows" {
fi, err := os.Stat(sockPath)
if err != nil {
t.Fatalf("os.Stat(%q) = %v", sockPath, err)
}

if fi.Mode()&os.ModeSocket == 0 {
t.Fatalf("%q is not a socket", sockPath)
}
fi, err := os.Stat(sockPath)
if err != nil {
t.Fatalf("os.Stat(%q) = %v", sockPath, err)
}

if fi.Mode()&os.ModeSocket == 0 {
t.Fatalf("%q is not a socket", sockPath)
}

// Try to connect to the socket.
Expand Down
7 changes: 1 addition & 6 deletions internal/socket/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,8 @@ import (
"strings"
)

// socketExists returns true if the socket path exists on linux and darwin
// on windows it always returns false, because of https://github.com/golang/go/issues/33357 (stat on sockets is broken on windows)
// socketExists returns true if the socket path exists
func socketExists(path string) (bool, error) {
if runtime.GOOS == "windows" {
return false, nil
}

_, err := os.Stat(path)
if err != nil {
if errors.Is(err, os.ErrNotExist) {
Expand Down
4 changes: 0 additions & 4 deletions jobapi/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,6 @@ func TestServerStartStop(t *testing.T) {
}

func TestServerStartStop_WithPreExistingSocket(t *testing.T) {
if runtime.GOOS == "windows" {
t.Skip("socket collision detection isn't support on windows. If the current go version is >1.23, it might be worth re-enabling this test, because hopefully the bug (https://github.com/golang/go/issues/33357) is fixed")
}

sockName := filepath.Join(os.TempDir(), "test-socket-collision.sock")
srv1, _, err := jobapi.NewServer(shell.TestingLogger{T: t}, sockName, env.New())
if err != nil {
Expand Down

0 comments on commit d4bbaf8

Please sign in to comment.