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 <aaron@tailscale.com>
This commit is contained in:
Aaron Klotz 2022-11-14 16:51:09 -07:00
parent d6021ae71c
commit 033bd94d4c
6 changed files with 20 additions and 11 deletions

View File

@ -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

View File

@ -29,6 +29,7 @@
"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 @@
"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 {

1
go.mod
View File

@ -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

2
go.sum
View File

@ -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=

View File

@ -11,6 +11,7 @@
// 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"

View File

@ -11,7 +11,6 @@
"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 {