Skip to content

Commit

Permalink
Use UDS for reproxy <-> depsscanner communciation on Windows
Browse files Browse the repository at this point in the history 
UDS is now supported in Windows and this change is intending to get rid
of TCP port communication since it seems mildly flaky on Windows CQ
machines for Chrome.

The UDS socket will be put inside the temp folder in Windows.

Bug: b/352206623
Tested: Ran the server on Windows and ensured we are able to connect to it
Change-Id: Ia9f80ec6ee7910ae53f1dccfce7bbc5b98568591
GitOrigin-RevId: 76543e3489c182fede7feed4c36abd66373e34ea
  • Loading branch information
@gkousik @copybara-github
gkousik authored and copybara-github committed Jul 29, 2024
1 parent 8976548 commit 2e412dd
Show file tree
Hide file tree
Showing 3 changed files with 122 additions and 63 deletions.
47 changes: 39 additions & 8 deletions internal/pkg/cppdependencyscanner/depsscannerclient/depsscannerclient.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,11 @@ import (
"errors"
"fmt"
"io"
"math/rand"
"net"
"os"
"path/filepath"
"runtime"
"strconv"
"strings"
"sync"
Expand Down Expand Up @@ -176,17 +178,46 @@ func New(ctx context.Context, executor executor, cacheDir string, cacheFileMaxMb
}

// buildAddress generates an address for the depsscanner process to listen on.
// If reproxy is on UNIX and using a UDS address then it will replace reproxy
// with depscan in the sock file name. If reproxy does not exist in the name, it
// will prepend _ds to the sock file name. Otherwise a random TCP port will be chosen.
// The path is constructed as:
//
// a) If platform is UNIX & using UDS for rewrapper <-> reproxy
// comms (e.g., /tmp/reproxy.sock), socket filename would be
// unix:///tmp/ds_depsscan.sock.
// b) If platform is Windows & using named pipe for rewrapper <-> reproxy
// comms (e.g., pipe://\\.pipe\reproxy.pipe), socket filename would be
// unix:<tmp-dir>\ds_<randnum>.sock (e.g., unix:c:\src\temp/ds_123.sock).
//
// If the socket path in (a) or (b) exceeds 108 characters, TCP ports will be used.
func buildAddress(proxyServerAddress string, openPortFunc func() (int, error)) (string, error) {
address := proxyServerAddress
if ipc.GrpcCxxSupportsUDS && strings.HasPrefix(address, "unix://") {
if strings.Contains(address, "reproxy") {
return strings.Replace(address, "reproxy", "depscan", -1), nil
if ipc.GrpcCxxSupportsUDS {
var socketFilePath string
switch {
case strings.HasPrefix(address, "unix://"):
if strings.Contains(address, "reproxy") {
return strings.Replace(address, "reproxy", "depscan", -1), nil
}
dir, sockFileName := filepath.Split(address[len("unix://"):])
socketFilePath = filepath.Join(dir, "ds_"+sockFileName)

case strings.HasPrefix(address, "pipe://"):
sockFileName := fmt.Sprintf("ds_%v.sock", rand.Intn(1000))
socketFilePath = filepath.Join(os.TempDir(), sockFileName)
}

sfLen := len(socketFilePath)
if sfLen > 0 && sfLen <= 108 {
if runtime.GOOS == "windows" {
// On Windows, for grpc C++, unix:// style socket file paths
// dont seem to work (i.e., the dependency scanner server in C++
// is not able to listen on that path).
return fmt.Sprintf("unix:%s", socketFilePath), nil
}
return fmt.Sprintf("unix://%s", socketFilePath), nil
}
if sfLen > 0 {
log.Warningf("buildAddress(%v), constructed socket file path exceeds 108 characters (%v). Falling back to using TCP ports.", proxyServerAddress, socketFilePath)
}
dir, sockFile := filepath.Split(address[len("unix://"):])
return fmt.Sprintf("unix://%s", filepath.Join(dir, "ds_"+sockFile)), nil
}
if strings.HasPrefix(address, "unix://") || strings.HasPrefix(address, "pipe://") {
address = fmt.Sprintf("%s:0", localhost)
Expand Down
105 changes: 53 additions & 52 deletions internal/pkg/cppdependencyscanner/depsscannerclient/depsscannerclient_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ package depsscannerclient
import (
"context"
"errors"
"fmt"
"os"
"path/filepath"
"runtime"
Expand Down Expand Up @@ -693,74 +694,74 @@ func TestFindKeyVal(t *testing.T) {

func TestBuildAddress(t *testing.T) {
tests := []struct {
name string
platforms []string
serverAddr string
openPortFunc func() (int, error)
wantAddr string
name string
platforms []string
serverAddr string
openPortFunc func() (int, error)
wantAddrPrefix string
}{
{
name: "UnixSocketReplacereproxy",
platforms: []string{"linux", "darwin"},
serverAddr: "unix:///some/dir/reproxy_123.sock",
openPortFunc: func() (int, error) { return 111, nil },
wantAddr: "unix:///some/dir/depscan_123.sock",
name: "UnixSocketReplacereproxy",
platforms: []string{"linux", "darwin"},
serverAddr: "unix:///some/dir/reproxy_123.sock",
openPortFunc: func() (int, error) { return 111, nil },
wantAddrPrefix: "unix:///some/dir/depscan_123.sock",
},
{
name: "UnixSocketAbsOnUnix",
platforms: []string{"linux", "darwin"},
serverAddr: "unix:///some/dir/somesocket.sock",
openPortFunc: func() (int, error) { return 111, nil },
wantAddr: "unix:///some/dir/ds_somesocket.sock",
name: "UnixSocketAbsOnUnix",
platforms: []string{"linux", "darwin"},
serverAddr: "unix:///some/dir/somesocket.sock",
openPortFunc: func() (int, error) { return 111, nil },
wantAddrPrefix: "unix:///some/dir/ds_somesocket.sock",
},
{
name: "UnixSocketAbsNoDirsOnUnix",
platforms: []string{"linux", "darwin"},
serverAddr: "unix:///somesocket.sock",
openPortFunc: func() (int, error) { return 111, nil },
wantAddr: "unix:///ds_somesocket.sock",
name: "UnixSocketAbsNoDirsOnUnix",
platforms: []string{"linux", "darwin"},
serverAddr: "unix:///somesocket.sock",
openPortFunc: func() (int, error) { return 111, nil },
wantAddrPrefix: "unix:///ds_somesocket.sock",
},
{
name: "UnixSocketRelNoDirsOnUnix",
platforms: []string{"linux", "darwin"},
serverAddr: "unix://somesocket.sock",
openPortFunc: func() (int, error) { return 111, nil },
wantAddr: "unix://ds_somesocket.sock",
name: "UnixSocketRelNoDirsOnUnix",
platforms: []string{"linux", "darwin"},
serverAddr: "unix://somesocket.sock",
openPortFunc: func() (int, error) { return 111, nil },
wantAddrPrefix: "unix://ds_somesocket.sock",
},
{
name: "UnixSocketRelOnUnix",
platforms: []string{"linux", "darwin"},
serverAddr: "unix://a/b/somesocket.sock",
openPortFunc: func() (int, error) { return 111, nil },
wantAddr: "unix://a/b/ds_somesocket.sock",
name: "UnixSocketRelOnUnix",
platforms: []string{"linux", "darwin"},
serverAddr: "unix://a/b/somesocket.sock",
openPortFunc: func() (int, error) { return 111, nil },
wantAddrPrefix: "unix://a/b/ds_somesocket.sock",
},
{
name: "UnixSocketOnWindows",
platforms: []string{"windows"},
serverAddr: "unix://some/dir/somesocket.sock",
openPortFunc: func() (int, error) { return 222, nil },
wantAddr: "127.0.0.1:222",
name: "UnixSocketOnWindows",
platforms: []string{"windows"},
serverAddr: "unix://C:\\src\\somesocket.sock",
openPortFunc: func() (int, error) { return 222, nil },
wantAddrPrefix: "unix:C:\\src\\ds_somesocket.sock",
},
{
name: "WindowsPipe",
platforms: []string{"windows"},
serverAddr: "pipe://pipename.pipe",
openPortFunc: func() (int, error) { return 333, nil },
wantAddr: "127.0.0.1:333",
name: "WindowsPipe",
platforms: []string{"windows"},
serverAddr: "pipe://pipename.pipe",
openPortFunc: func() (int, error) { return 333, nil },
wantAddrPrefix: fmt.Sprintf("unix:%s", filepath.Join(os.TempDir(), "ds")),
},
{
name: "TCP",
platforms: []string{"linux", "darwin", "windows"},
serverAddr: "127.0.0.1:8000",
openPortFunc: func() (int, error) { return 444, nil },
wantAddr: "127.0.0.1:444",
name: "TCP",
platforms: []string{"linux", "darwin", "windows"},
serverAddr: "127.0.0.1:8000",
openPortFunc: func() (int, error) { return 444, nil },
wantAddrPrefix: "127.0.0.1:444",
},
{
name: "AnotherTCPAddress",
platforms: []string{"linux", "darwin", "windows"},
serverAddr: "192.168.1.1:8000",
openPortFunc: func() (int, error) { return 555, nil },
wantAddr: "192.168.1.1:555",
name: "AnotherTCPAddress",
platforms: []string{"linux", "darwin", "windows"},
serverAddr: "192.168.1.1:8000",
openPortFunc: func() (int, error) { return 555, nil },
wantAddrPrefix: "192.168.1.1:555",
},
}
for _, test := range tests {
Expand All @@ -771,8 +772,8 @@ func TestBuildAddress(t *testing.T) {
if err != nil {
t.Errorf("buildAddress(%v) returned unexpected error: %v", test.serverAddr, err)
}
if gotAddr != test.wantAddr {
t.Errorf("buildAddress(%v) returned wrong address, wanted '%v', got '%v'", test.serverAddr, test.wantAddr, gotAddr)
if !strings.HasPrefix(gotAddr, test.wantAddrPrefix) {
t.Errorf("buildAddress(%v) returned wrong address, wanted prefix '%v', got '%v'", test.serverAddr, test.wantAddrPrefix, gotAddr)
}
})
}
Expand Down
33 changes: 30 additions & 3 deletions internal/pkg/ipc/features_windows.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,34 @@

package ipc

const (
// GrpcCxxSupportsUDS is false because of https://github.com/grpc/grpc/issues/13447 and https://github.com/grpc/grpc/issues/22285.
GrpcCxxSupportsUDS = false
import (
"sync"
"syscall"
"unsafe"
)

// This function is copied over from an internal package in Go
// https://github.com/golang/go/blob/aa97a012b4be393c1725c16a78b92dea81632378/src/internal/syscall/windows/version_windows.go#L95
var supportUnixSocket = sync.OnceValue(func() bool {
var size uint32
// First call to get the required buffer size in bytes.
// Ignore the error, it will always fail.
_, _ = syscall.WSAEnumProtocols(nil, nil, &size)
n := int32(size) / int32(unsafe.Sizeof(syscall.WSAProtocolInfo{}))
// Second call to get the actual protocols.
buf := make([]syscall.WSAProtocolInfo, n)
n, err := syscall.WSAEnumProtocols(nil, &buf[0], &size)
if err != nil {
return false
}
for i := int32(0); i < n; i++ {
if buf[i].AddressFamily == syscall.AF_UNIX {
return true
}
}
return false
})

var (
GrpcCxxSupportsUDS = supportUnixSocket()
)

0 comments on commit 2e412dd

Please sign in to comment.