From 033bd94d4ca811de4243aca17b6670e95c503807 Mon Sep 17 00:00:00 2001 From: Aaron Klotz Date: Mon, 14 Nov 2022 16:51:09 -0700 Subject: [PATCH] cmd/tailscaled, wgengine/router: use wingoes/com for COM initialization instead of go-ole This patch removes the crappy, half-backed COM initialization used by `go-ole` and replaces that with the `StartRuntime` function from `wingoes`, a library I have started which, among other things, initializes COM properly. In particular, we should always be initializing COM to use the multithreaded apartment. Every single OS thread in the process becomes implicitly initialized as part of the MTA, so we do not need to concern ourselves as to whether or not any particular OS thread has initialized COM. Furthermore, we no longer need to lock the OS thread when calling methods on COM interfaces. Single-threaded apartments are designed solely for working with Win32 threads that have a message pump; any other use of the STA is invalid. Fixes https://github.com/tailscale/tailscale/issues/3137 Signed-off-by: Aaron Klotz --- cmd/tailscaled/depaware.txt | 2 ++ cmd/tailscaled/tailscaled_windows.go | 12 ++++++++++++ go.mod | 1 + go.sum | 2 ++ tstest/integration/tailscaled_deps_test_windows.go | 1 + wgengine/router/ifconfig_windows.go | 13 ++----------- 6 files changed, 20 insertions(+), 11 deletions(-) diff --git a/cmd/tailscaled/depaware.txt b/cmd/tailscaled/depaware.txt index e842e894a..78913fc62 100644 --- a/cmd/tailscaled/depaware.txt +++ b/cmd/tailscaled/depaware.txt @@ -64,6 +64,8 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de L github.com/aws/smithy-go/waiter from github.com/aws/aws-sdk-go-v2/service/ssm L github.com/coreos/go-iptables/iptables from tailscale.com/wgengine/router LD 💣 github.com/creack/pty from tailscale.com/ssh/tailssh + W 💣 github.com/dblohm7/wingoes from github.com/dblohm7/wingoes/com + W 💣 github.com/dblohm7/wingoes/com from tailscale.com/cmd/tailscaled github.com/fxamacker/cbor/v2 from tailscale.com/tka W 💣 github.com/go-ole/go-ole from github.com/go-ole/go-ole/oleutil+ W 💣 github.com/go-ole/go-ole/oleutil from tailscale.com/wgengine/winnet diff --git a/cmd/tailscaled/tailscaled_windows.go b/cmd/tailscaled/tailscaled_windows.go index ae47769c0..2f3296e40 100644 --- a/cmd/tailscaled/tailscaled_windows.go +++ b/cmd/tailscaled/tailscaled_windows.go @@ -29,6 +29,7 @@ import ( "os" "time" + "github.com/dblohm7/wingoes/com" "golang.org/x/sys/windows" "golang.org/x/sys/windows/svc" "golang.org/x/sys/windows/svc/eventlog" @@ -51,6 +52,17 @@ import ( "tailscale.com/wgengine/router" ) +func init() { + // Initialize COM process-wide. + comProcessType := com.Service + if !isWindowsService() { + comProcessType = com.ConsoleApp + } + if err := com.StartRuntime(comProcessType); err != nil { + log.Printf("wingoes.com.StartRuntime(%d) failed: %v", comProcessType, err) + } +} + const serviceName = "Tailscale" func isWindowsService() bool { diff --git a/go.mod b/go.mod index ae0517619..74c6f08ce 100644 --- a/go.mod +++ b/go.mod @@ -17,6 +17,7 @@ require ( github.com/coreos/go-systemd v0.0.0-20191104093116-d3cd4ed1dbcf github.com/creack/pty v1.1.17 github.com/dave/jennifer v1.4.1 + github.com/dblohm7/wingoes v0.0.0-20221124203957-6ac47ab19aa5 github.com/dsnet/try v0.0.3 github.com/evanw/esbuild v0.14.53 github.com/frankban/quicktest v1.14.0 diff --git a/go.sum b/go.sum index f812df4df..ee4121ec0 100644 --- a/go.sum +++ b/go.sum @@ -248,6 +248,8 @@ github.com/davecgh/go-spew v0.0.0-20161028175848-04cdfd42973b/go.mod h1:J7Y8YcW2 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/dblohm7/wingoes v0.0.0-20221124203957-6ac47ab19aa5 h1:84SSlQpWqllOmtng34NorWGJbzX00SI2J4MQjXNYUuU= +github.com/dblohm7/wingoes v0.0.0-20221124203957-6ac47ab19aa5/go.mod h1:LPBSRY0diEb4/R1gqa4OaBexvmklv7XdPv7m6cudDR8= github.com/denis-tingajkin/go-header v0.3.1/go.mod h1:sq/2IxMhaZX+RRcgHfCRx/m0M5na0fBt4/CRe7Lrji0= github.com/denis-tingajkin/go-header v0.4.2 h1:jEeSF4sdv8/3cT/WY8AgDHUoItNSoEZ7qg9dX7pc218= github.com/denis-tingajkin/go-header v0.4.2/go.mod h1:eLRHAVXzE5atsKAnNRDB90WHCFFnBUn4RN0nRcs1LJA= diff --git a/tstest/integration/tailscaled_deps_test_windows.go b/tstest/integration/tailscaled_deps_test_windows.go index 122c99528..b00d59b15 100644 --- a/tstest/integration/tailscaled_deps_test_windows.go +++ b/tstest/integration/tailscaled_deps_test_windows.go @@ -11,6 +11,7 @@ import ( // Otherwise cmd/go never sees that we depend on these packages' // transitive deps when we run "go install tailscaled" in a child // process and can cache a prior success when a dependency changes. + _ "github.com/dblohm7/wingoes/com" _ "golang.org/x/sys/windows" _ "golang.org/x/sys/windows/svc" _ "golang.org/x/sys/windows/svc/eventlog" diff --git a/wgengine/router/ifconfig_windows.go b/wgengine/router/ifconfig_windows.go index 97a7acb3f..337815d91 100644 --- a/wgengine/router/ifconfig_windows.go +++ b/wgengine/router/ifconfig_windows.go @@ -11,7 +11,6 @@ import ( "log" "net" "net/netip" - "runtime" "sort" "time" @@ -178,17 +177,9 @@ func setPrivateNetwork(ifcLUID winipcfg.LUID) (bool, error) { return false, fmt.Errorf("ifcLUID.GUID: %v", err) } - // Lock OS thread when using OLE, which seems to be a requirement - // from the Microsoft docs. go-ole doesn't seem to handle it automatically. - // https://github.com/tailscale/tailscale/issues/921#issuecomment-727526807 - runtime.LockOSThread() - defer runtime.UnlockOSThread() - + // aaron: DO NOT call Initialize() or Uninitialize() on c! + // We've already handled that process-wide. var c ole.Connection - if err := c.Initialize(); err != nil { - return false, fmt.Errorf("c.Initialize: %v", err) - } - defer c.Uninitialize() m, err := winnet.NewNetworkListManager(&c) if err != nil {