From 06b092388e4efb2226a264a03df14b778505278c Mon Sep 17 00:00:00 2001 From: "M. J. Fromberger" Date: Wed, 29 Oct 2025 08:37:19 -0700 Subject: [PATCH] ipn/ipnlocal: do not stall event processing for appc route updates (#17663) A follow-up to #17411. Put AppConnector events into a task queue, as they may take some time to process. Ensure that the queue is stopped at shutdown so that cleanup will remain orderly. Because events are delivered on a separate goroutine, slow processing of an event does not cause an immediate problem; however, a subscriber that blocks for a long time will push back on the bus as a whole. See https://godoc.org/tailscale.com/util/eventbus#hdr-Expected_subscriber_behavior for more discussion. Updates #17192 Updates #15160 Change-Id: Ib313cc68aec273daf2b1ad79538266c81ef063e3 Signed-off-by: M. J. Fromberger --- ipn/ipnlocal/local.go | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 7b2257cca..df278a325 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -87,6 +87,7 @@ import ( "tailscale.com/util/clientmetric" "tailscale.com/util/dnsname" "tailscale.com/util/eventbus" + "tailscale.com/util/execqueue" "tailscale.com/util/goroutines" "tailscale.com/util/mak" "tailscale.com/util/osuser" @@ -187,6 +188,7 @@ type LocalBackend struct { statsLogf logger.Logf // for printing peers stats on change sys *tsd.System eventClient *eventbus.Client + appcTask execqueue.ExecQueue // handles updates from appc health *health.Tracker // always non-nil polc policyclient.Client // always non-nil @@ -613,12 +615,14 @@ func (b *LocalBackend) onAppConnectorRouteUpdate(ru appctype.RouteUpdate) { // We need to find a way to ensure that changes to the backend state are applied // consistently in the presnce of profile changes, which currently may not happen in // a single atomic step. See: https://github.com/tailscale/tailscale/issues/17414 - if err := b.AdvertiseRoute(ru.Advertise...); err != nil { - b.logf("appc: failed to advertise routes: %v: %v", ru.Advertise, err) - } - if err := b.UnadvertiseRoute(ru.Unadvertise...); err != nil { - b.logf("appc: failed to unadvertise routes: %v: %v", ru.Unadvertise, err) - } + b.appcTask.Add(func() { + if err := b.AdvertiseRoute(ru.Advertise...); err != nil { + b.logf("appc: failed to advertise routes: %v: %v", ru.Advertise, err) + } + if err := b.UnadvertiseRoute(ru.Unadvertise...); err != nil { + b.logf("appc: failed to unadvertise routes: %v: %v", ru.Unadvertise, err) + } + }) } func (b *LocalBackend) onAppConnectorStoreRoutes(ri appctype.RouteInfo) { @@ -1082,6 +1086,7 @@ func (b *LocalBackend) Shutdown() { // 1. Event handlers also acquire b.mu, they can deadlock with c.Shutdown(). // 2. Event handlers may not guard against undesirable post/in-progress // LocalBackend.Shutdown() behaviors. + b.appcTask.Shutdown() b.eventClient.Close() b.em.close()