Skip to content

Commit fe1ba7d

Browse files
dblohm7coadler
authored andcommitted
cmd/tailscaled: change Windows implementation to shut down subprocess via closing its stdin
We've been doing a hard kill of the subprocess, which is only safe as long as both the cli and gui are not running and the subprocess has had the opportunity to clean up DNS settings etc. If unattended mode is turned on, this is definitely unsafe. I changed babysitProc to close the subprocess's stdin to make it shut down, and then I plumbed a cancel function into the stdin reader on the subprocess side. Fixes https://github.com/tailscale/corp/issues/5621 Signed-off-by: Aaron Klotz <[email protected]>
1 parent b2d4b43 commit fe1ba7d

File tree

1 file changed

+21
-8
lines changed

1 file changed

+21
-8
lines changed

cmd/tailscaled/tailscaled_windows.go

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525
"encoding/json"
2626
"errors"
2727
"fmt"
28+
"io"
2829
"log"
2930
"net/netip"
3031
"os"
@@ -273,17 +274,24 @@ func beWindowsSubprocess() bool {
273274
log.Printf("Error reading environment config: %v", err)
274275
}
275276

277+
ctx, cancel := context.WithCancel(context.Background())
276278
go func() {
277279
b := make([]byte, 16)
278280
for {
279281
_, err := os.Stdin.Read(b)
282+
if err == io.EOF {
283+
// Parent wants us to shut down gracefully.
284+
log.Printf("subproc received EOF from stdin")
285+
cancel()
286+
return
287+
}
280288
if err != nil {
281289
log.Fatalf("stdin err (parent process died): %v", err)
282290
}
283291
}
284292
}()
285293

286-
err := startIPNServer(context.Background(), log.Printf, logid)
294+
err := startIPNServer(ctx, log.Printf, logid)
287295
if err != nil {
288296
log.Fatalf("ipnserver: %v", err)
289297
}
@@ -365,8 +373,9 @@ func babysitProc(ctx context.Context, args []string, logf logger.Logf) {
365373
}
366374

367375
var proc struct {
368-
mu sync.Mutex
369-
p *os.Process
376+
mu sync.Mutex
377+
p *os.Process
378+
wStdin *os.File
370379
}
371380

372381
done := make(chan struct{})
@@ -378,15 +387,18 @@ func babysitProc(ctx context.Context, args []string, logf logger.Logf) {
378387
case sig = <-interrupt:
379388
logf("babysitProc: got signal: %v", sig)
380389
close(done)
390+
proc.mu.Lock()
391+
proc.p.Signal(sig)
392+
proc.mu.Unlock()
381393
case <-ctx.Done():
382394
logf("babysitProc: context done")
383-
sig = os.Kill
384395
close(done)
396+
proc.mu.Lock()
397+
// Closing wStdin gives the subprocess a chance to shut down cleanly,
398+
// which is important for cleaning up DNS settings etc.
399+
proc.wStdin.Close()
400+
proc.mu.Unlock()
385401
}
386-
387-
proc.mu.Lock()
388-
proc.p.Signal(sig)
389-
proc.mu.Unlock()
390402
}()
391403

392404
bo := backoff.NewBackoff("babysitProc", logf, 30*time.Second)
@@ -448,6 +460,7 @@ func babysitProc(ctx context.Context, args []string, logf logger.Logf) {
448460
} else {
449461
proc.mu.Lock()
450462
proc.p = cmd.Process
463+
proc.wStdin = wStdin
451464
proc.mu.Unlock()
452465

453466
err = cmd.Wait()

0 commit comments

Comments
 (0)