fix(daemon): harden parked lease teardown
- commit
- 52935c6fe4fb23ae5aa207d46e497b3054fdf3de
- author
- brice · · 2026-08-25 15:46 UTC
- parent
- b4a3ff1
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +38 -9
M internal/usecase/daemon/parked_route.go +15 -9
| @@ -24,6 +24,10 @@ type parkedRouteLease struct { | |||
| 24 | 24 | expired bool | |
| 25 | 25 | } | |
| 26 | 26 | ||
| 27 | + | func (lease *parkedRouteLease) matches(id ports.ParkedRouteLeaseID, generation uint64, transport transportSnapshot) bool { | |
| 28 | + | return lease != nil && lease.id == id && lease.generation == generation && lease.transport == transport | |
| 29 | + | } | |
| 30 | + | ||
| 27 | 31 | func newParkedRouteLeaseID() (ports.ParkedRouteLeaseID, error) { | |
| 28 | 32 | var id ports.ParkedRouteLeaseID | |
| 29 | 33 | if _, err := io.ReadFull(rand.Reader, id[:]); err != nil { | |
| @@ -96,9 +100,7 @@ func (d *Daemon) expireParkedRoute(ac *attachedClient, lease *parkedRouteLease) | |||
| 96 | 100 | return | |
| 97 | 101 | } | |
| 98 | 102 | ac.parkedRoute = nil | |
| 99 | - | if !lease.suspended { | |
| 100 | - | ac.parkedRouteOutput.Store(false) | |
| 101 | - | } | |
| 103 | + | ac.parkedRouteOutput.Store(false) | |
| 102 | 104 | ac.parkedRouteFullPending.Store(false) | |
| 103 | 105 | ac.parkedRouteMu.Unlock() | |
| 104 | 106 | if ac.connectionGeneration.Load() == lease.generation && ac.transportSnapshotCurrent(lease.transport) { | |
| @@ -125,7 +127,7 @@ func (ac *attachedClient) parkedRouteStatus(token attachmentConnectionToken, id | |||
| 125 | 127 | ac.parkedRouteMu.Lock() | |
| 126 | 128 | defer ac.parkedRouteMu.Unlock() | |
| 127 | 129 | lease := ac.parkedRoute | |
| 128 | - | if lease == nil || lease.id != id || lease.generation != token.generation || lease.transport != token.transport { | |
| 130 | + | if !lease.matches(id, token.generation, token.transport) { | |
| 129 | 131 | return ports.ParkedRouteRejected | |
| 130 | 132 | } | |
| 131 | 133 | if !now.Before(lease.expiresAt) { | |
| @@ -148,7 +150,7 @@ func (ac *attachedClient) prepareParkedRoute(token attachmentConnectionToken, id | |||
| 148 | 150 | ac.parkedRouteMu.Lock() | |
| 149 | 151 | defer ac.parkedRouteMu.Unlock() | |
| 150 | 152 | lease := ac.parkedRoute | |
| 151 | - | if lease == nil || lease.id != id || lease.generation != token.generation || lease.transport != token.transport || lease.suspended { | |
| 153 | + | if !lease.matches(id, token.generation, token.transport) || lease.suspended { | |
| 152 | 154 | return ports.ParkedRouteRejected | |
| 153 | 155 | } | |
| 154 | 156 | lease.suspended = true | |
| @@ -163,7 +165,7 @@ func (ac *attachedClient) consumeParkedRoute(token attachmentConnectionToken, id | |||
| 163 | 165 | ac.parkedRouteMu.Lock() | |
| 164 | 166 | defer ac.parkedRouteMu.Unlock() | |
| 165 | 167 | lease := ac.parkedRoute | |
| 166 | - | if lease == nil || lease.id != id || lease.generation != token.generation || lease.transport != token.transport || !lease.suspended { | |
| 168 | + | if !lease.matches(id, token.generation, token.transport) || !lease.suspended || lease.inFlight { | |
| 167 | 169 | return ports.ParkedRouteRejected | |
| 168 | 170 | } | |
| 169 | 171 | stopParkedRouteLeaseLocked(lease) | |
| @@ -178,12 +180,14 @@ func (ac *attachedClient) beginParkedRouteSwitch(token attachmentConnectionToken | |||
| 178 | 180 | ac.parkedRouteMu.Lock() | |
| 179 | 181 | defer ac.parkedRouteMu.Unlock() | |
| 180 | 182 | lease := ac.parkedRoute | |
| 181 | - | if lease == nil || lease.id != id || lease.generation != token.generation || lease.transport != token.transport || !lease.suspended || lease.inFlight { | |
| 183 | + | if !lease.matches(id, token.generation, token.transport) || !lease.suspended || lease.inFlight { | |
| 182 | 184 | return ports.ParkedRouteRejected | |
| 183 | 185 | } | |
| 184 | 186 | if lease.expired || !now.Before(lease.expiresAt) { | |
| 185 | 187 | stopParkedRouteLeaseLocked(lease) | |
| 186 | 188 | ac.parkedRoute = nil | |
| 189 | + | ac.parkedRouteOutput.Store(false) | |
| 190 | + | ac.parkedRouteFullPending.Store(false) | |
| 187 | 191 | return ports.ParkedRouteExpired | |
| 188 | 192 | } | |
| 189 | 193 | lease.inFlight = true | |
| @@ -197,13 +201,15 @@ func (ac *attachedClient) rejectParkedRouteSwitch(token attachmentConnectionToke | |||
| 197 | 201 | ac.parkedRouteMu.Lock() | |
| 198 | 202 | defer ac.parkedRouteMu.Unlock() | |
| 199 | 203 | lease := ac.parkedRoute | |
| 200 | - | if lease == nil || lease.id != id || lease.generation != token.generation || lease.transport != token.transport || !lease.inFlight { | |
| 204 | + | if !lease.matches(id, token.generation, token.transport) || !lease.inFlight { | |
| 201 | 205 | return ports.ParkedRouteRejected | |
| 202 | 206 | } | |
| 203 | 207 | lease.inFlight = false | |
| 204 | 208 | if lease.expired || !now.Before(lease.expiresAt) { | |
| 205 | 209 | stopParkedRouteLeaseLocked(lease) | |
| 206 | 210 | ac.parkedRoute = nil | |
| 211 | + | ac.parkedRouteOutput.Store(false) | |
| 212 | + | ac.parkedRouteFullPending.Store(false) | |
| 207 | 213 | return ports.ParkedRouteExpired | |
| 208 | 214 | } | |
| 209 | 215 | return 0 | |
| @@ -216,7 +222,7 @@ func (ac *attachedClient) consumePublishedParkedRoute(previous, published attach | |||
| 216 | 222 | ac.parkedRouteMu.Lock() | |
| 217 | 223 | defer ac.parkedRouteMu.Unlock() | |
| 218 | 224 | lease := ac.parkedRoute | |
| 219 | - | if lease == nil || lease.id != id || lease.generation != previous.generation || lease.transport != previous.transport || !lease.suspended || !lease.inFlight { | |
| 225 | + | if !lease.matches(id, previous.generation, previous.transport) || !lease.suspended || !lease.inFlight { | |
| 220 | 226 | return ports.ParkedRouteRejected | |
| 221 | 227 | } | |
| 222 | 228 | // Expiry is checked when the request is admitted. Once publication starts, | |
| @@ -24,6 +24,10 @@ type parkedRouteLease struct { | |||||
| 24 | expired bool | 24 | expired bool | ||
| 25 | } | 25 | } | ||
| 26 | 26 | ||||
| 27 | + | func (lease *parkedRouteLease) matches(id ports.ParkedRouteLeaseID, generation uint64, transport transportSnapshot) bool { | |||
| 28 | + | return lease != nil && lease.id == id && lease.generation == generation && lease.transport == transport | |||
| 29 | + | } | |||
| 30 | + | ||||
| 27 | func newParkedRouteLeaseID() (ports.ParkedRouteLeaseID, error) { | 31 | func newParkedRouteLeaseID() (ports.ParkedRouteLeaseID, error) { | ||
| 28 | var id ports.ParkedRouteLeaseID | 32 | var id ports.ParkedRouteLeaseID | ||
| 29 | if _, err := io.ReadFull(rand.Reader, id[:]); err != nil { | 33 | if _, err := io.ReadFull(rand.Reader, id[:]); err != nil { | ||
| @@ -96,9 +100,7 @@ func (d *Daemon) expireParkedRoute(ac *attachedClient, lease *parkedRouteLease) | |||||
| 96 | return | 100 | return | ||
| 97 | } | 101 | } | ||
| 98 | ac.parkedRoute = nil | 102 | ac.parkedRoute = nil | ||
| 99 | - | if !lease.suspended { | 103 | + | ac.parkedRouteOutput.Store(false) |
| 100 | - | ac.parkedRouteOutput.Store(false) | |||
| 101 | - | } | |||
| 102 | ac.parkedRouteFullPending.Store(false) | 104 | ac.parkedRouteFullPending.Store(false) | ||
| 103 | ac.parkedRouteMu.Unlock() | 105 | ac.parkedRouteMu.Unlock() | ||
| 104 | if ac.connectionGeneration.Load() == lease.generation && ac.transportSnapshotCurrent(lease.transport) { | 106 | if ac.connectionGeneration.Load() == lease.generation && ac.transportSnapshotCurrent(lease.transport) { | ||
| @@ -125,7 +127,7 @@ func (ac *attachedClient) parkedRouteStatus(token attachmentConnectionToken, id | |||||
| 125 | ac.parkedRouteMu.Lock() | 127 | ac.parkedRouteMu.Lock() | ||
| 126 | defer ac.parkedRouteMu.Unlock() | 128 | defer ac.parkedRouteMu.Unlock() | ||
| 127 | lease := ac.parkedRoute | 129 | lease := ac.parkedRoute | ||
| 128 | - | if lease == nil || lease.id != id || lease.generation != token.generation || lease.transport != token.transport { | 130 | + | if !lease.matches(id, token.generation, token.transport) { |
| 129 | return ports.ParkedRouteRejected | 131 | return ports.ParkedRouteRejected | ||
| 130 | } | 132 | } | ||
| 131 | if !now.Before(lease.expiresAt) { | 133 | if !now.Before(lease.expiresAt) { | ||
| @@ -148,7 +150,7 @@ func (ac *attachedClient) prepareParkedRoute(token attachmentConnectionToken, id | |||||
| 148 | ac.parkedRouteMu.Lock() | 150 | ac.parkedRouteMu.Lock() | ||
| 149 | defer ac.parkedRouteMu.Unlock() | 151 | defer ac.parkedRouteMu.Unlock() | ||
| 150 | lease := ac.parkedRoute | 152 | lease := ac.parkedRoute | ||
| 151 | - | if lease == nil || lease.id != id || lease.generation != token.generation || lease.transport != token.transport || lease.suspended { | 153 | + | if !lease.matches(id, token.generation, token.transport) || lease.suspended { |
| 152 | return ports.ParkedRouteRejected | 154 | return ports.ParkedRouteRejected | ||
| 153 | } | 155 | } | ||
| 154 | lease.suspended = true | 156 | lease.suspended = true | ||
| @@ -163,7 +165,7 @@ func (ac *attachedClient) consumeParkedRoute(token attachmentConnectionToken, id | |||||
| 163 | ac.parkedRouteMu.Lock() | 165 | ac.parkedRouteMu.Lock() | ||
| 164 | defer ac.parkedRouteMu.Unlock() | 166 | defer ac.parkedRouteMu.Unlock() | ||
| 165 | lease := ac.parkedRoute | 167 | lease := ac.parkedRoute | ||
| 166 | - | if lease == nil || lease.id != id || lease.generation != token.generation || lease.transport != token.transport || !lease.suspended { | 168 | + | if !lease.matches(id, token.generation, token.transport) || !lease.suspended || lease.inFlight { |
| 167 | return ports.ParkedRouteRejected | 169 | return ports.ParkedRouteRejected | ||
| 168 | } | 170 | } | ||
| 169 | stopParkedRouteLeaseLocked(lease) | 171 | stopParkedRouteLeaseLocked(lease) | ||
| @@ -178,12 +180,14 @@ func (ac *attachedClient) beginParkedRouteSwitch(token attachmentConnectionToken | |||||
| 178 | ac.parkedRouteMu.Lock() | 180 | ac.parkedRouteMu.Lock() | ||
| 179 | defer ac.parkedRouteMu.Unlock() | 181 | defer ac.parkedRouteMu.Unlock() | ||
| 180 | lease := ac.parkedRoute | 182 | lease := ac.parkedRoute | ||
| 181 | - | if lease == nil || lease.id != id || lease.generation != token.generation || lease.transport != token.transport || !lease.suspended || lease.inFlight { | 183 | + | if !lease.matches(id, token.generation, token.transport) || !lease.suspended || lease.inFlight { |
| 182 | return ports.ParkedRouteRejected | 184 | return ports.ParkedRouteRejected | ||
| 183 | } | 185 | } | ||
| 184 | if lease.expired || !now.Before(lease.expiresAt) { | 186 | if lease.expired || !now.Before(lease.expiresAt) { | ||
| 185 | stopParkedRouteLeaseLocked(lease) | 187 | stopParkedRouteLeaseLocked(lease) | ||
| 186 | ac.parkedRoute = nil | 188 | ac.parkedRoute = nil | ||
| 189 | + | ac.parkedRouteOutput.Store(false) | |||
| 190 | + | ac.parkedRouteFullPending.Store(false) | |||
| 187 | return ports.ParkedRouteExpired | 191 | return ports.ParkedRouteExpired | ||
| 188 | } | 192 | } | ||
| 189 | lease.inFlight = true | 193 | lease.inFlight = true | ||
| @@ -197,13 +201,15 @@ func (ac *attachedClient) rejectParkedRouteSwitch(token attachmentConnectionToke | |||||
| 197 | ac.parkedRouteMu.Lock() | 201 | ac.parkedRouteMu.Lock() | ||
| 198 | defer ac.parkedRouteMu.Unlock() | 202 | defer ac.parkedRouteMu.Unlock() | ||
| 199 | lease := ac.parkedRoute | 203 | lease := ac.parkedRoute | ||
| 200 | - | if lease == nil || lease.id != id || lease.generation != token.generation || lease.transport != token.transport || !lease.inFlight { | 204 | + | if !lease.matches(id, token.generation, token.transport) || !lease.inFlight { |
| 201 | return ports.ParkedRouteRejected | 205 | return ports.ParkedRouteRejected | ||
| 202 | } | 206 | } | ||
| 203 | lease.inFlight = false | 207 | lease.inFlight = false | ||
| 204 | if lease.expired || !now.Before(lease.expiresAt) { | 208 | if lease.expired || !now.Before(lease.expiresAt) { | ||
| 205 | stopParkedRouteLeaseLocked(lease) | 209 | stopParkedRouteLeaseLocked(lease) | ||
| 206 | ac.parkedRoute = nil | 210 | ac.parkedRoute = nil | ||
| 211 | + | ac.parkedRouteOutput.Store(false) | |||
| 212 | + | ac.parkedRouteFullPending.Store(false) | |||
| 207 | return ports.ParkedRouteExpired | 213 | return ports.ParkedRouteExpired | ||
| 208 | } | 214 | } | ||
| 209 | return 0 | 215 | return 0 | ||
| @@ -216,7 +222,7 @@ func (ac *attachedClient) consumePublishedParkedRoute(previous, published attach | |||||
| 216 | ac.parkedRouteMu.Lock() | 222 | ac.parkedRouteMu.Lock() | ||
| 217 | defer ac.parkedRouteMu.Unlock() | 223 | defer ac.parkedRouteMu.Unlock() | ||
| 218 | lease := ac.parkedRoute | 224 | lease := ac.parkedRoute | ||
| 219 | - | if lease == nil || lease.id != id || lease.generation != previous.generation || lease.transport != previous.transport || !lease.suspended || !lease.inFlight { | 225 | + | if !lease.matches(id, previous.generation, previous.transport) || !lease.suspended || !lease.inFlight { |
| 220 | return ports.ParkedRouteRejected | 226 | return ports.ParkedRouteRejected | ||
| 221 | } | 227 | } | ||
| 222 | // Expiry is checked when the request is admitted. Once publication starts, | 228 | // Expiry is checked when the request is admitted. Once publication starts, | ||
M internal/usecase/daemon/parked_route_test.go +23 -0
| @@ -279,6 +279,24 @@ func TestParkedRouteExpiredLeaseFailsClosed(t *testing.T) { | |||
| 279 | 279 | require.False(t, ac.parkedRouteOutput.Load()) | |
| 280 | 280 | } | |
| 281 | 281 | ||
| 282 | + | func TestParkedRouteResumeRejectsSwitchInFlight(t *testing.T) { | |
| 283 | + | d, source, ac, sends, releases := newManualTabSession(t, 1) | |
| 284 | + | defer releaseAll(releases) | |
| 285 | + | ac.navigationCapabilities = ports.NavigationCapabilityHomePicker | |
| 286 | + | leaseID := armAndPrepareParkedRoute(t, d, source, ac, sends) | |
| 287 | + | ||
| 288 | + | ac.parkedRouteMu.Lock() | |
| 289 | + | ac.parkedRoute.inFlight = true | |
| 290 | + | ac.parkedRouteMu.Unlock() | |
| 291 | + | status := ac.consumeParkedRoute(source.attachmentToken(ac, ac.transport()), leaseID, d.clock.Now()) | |
| 292 | + | ||
| 293 | + | require.Equal(t, ports.ParkedRouteRejected, status) | |
| 294 | + | ac.parkedRouteMu.Lock() | |
| 295 | + | require.NotNil(t, ac.parkedRoute) | |
| 296 | + | ac.parkedRouteMu.Unlock() | |
| 297 | + | require.True(t, ac.parkedRouteOutput.Load()) | |
| 298 | + | } | |
| 299 | + | ||
| 282 | 300 | func TestParkedRouteLeaseExpiryClosesExactRetainedTransport(t *testing.T) { | |
| 283 | 301 | d, source, ac, _, releases := newManualTabSession(t, 1) | |
| 284 | 302 | defer releaseAll(releases) | |
| @@ -301,6 +319,11 @@ func TestParkedRouteLeaseExpiryClosesExactRetainedTransport(t *testing.T) { | |||
| 301 | 319 | case <-time.After(time.Second): | |
| 302 | 320 | t.Fatal("parked transport remained open after its lease expired") | |
| 303 | 321 | } | |
| 322 | + | ac.parkedRouteMu.Lock() | |
| 323 | + | require.Nil(t, ac.parkedRoute) | |
| 324 | + | ac.parkedRouteMu.Unlock() | |
| 325 | + | require.False(t, ac.parkedRouteOutput.Load()) | |
| 326 | + | require.False(t, ac.parkedRouteFullPending.Load()) | |
| 304 | 327 | } | |
| 305 | 328 | ||
| 306 | 329 | func TestParkedRouteMalformedSwitchRequestsFailClosed(t *testing.T) { | |
| @@ -279,6 +279,24 @@ func TestParkedRouteExpiredLeaseFailsClosed(t *testing.T) { | |||||
| 279 | require.False(t, ac.parkedRouteOutput.Load()) | 279 | require.False(t, ac.parkedRouteOutput.Load()) | ||
| 280 | } | 280 | } | ||
| 281 | 281 | ||||
| 282 | + | func TestParkedRouteResumeRejectsSwitchInFlight(t *testing.T) { | |||
| 283 | + | d, source, ac, sends, releases := newManualTabSession(t, 1) | |||
| 284 | + | defer releaseAll(releases) | |||
| 285 | + | ac.navigationCapabilities = ports.NavigationCapabilityHomePicker | |||
| 286 | + | leaseID := armAndPrepareParkedRoute(t, d, source, ac, sends) | |||
| 287 | + | ||||
| 288 | + | ac.parkedRouteMu.Lock() | |||
| 289 | + | ac.parkedRoute.inFlight = true | |||
| 290 | + | ac.parkedRouteMu.Unlock() | |||
| 291 | + | status := ac.consumeParkedRoute(source.attachmentToken(ac, ac.transport()), leaseID, d.clock.Now()) | |||
| 292 | + | ||||
| 293 | + | require.Equal(t, ports.ParkedRouteRejected, status) | |||
| 294 | + | ac.parkedRouteMu.Lock() | |||
| 295 | + | require.NotNil(t, ac.parkedRoute) | |||
| 296 | + | ac.parkedRouteMu.Unlock() | |||
| 297 | + | require.True(t, ac.parkedRouteOutput.Load()) | |||
| 298 | + | } | |||
| 299 | + | ||||
| 282 | func TestParkedRouteLeaseExpiryClosesExactRetainedTransport(t *testing.T) { | 300 | func TestParkedRouteLeaseExpiryClosesExactRetainedTransport(t *testing.T) { | ||
| 283 | d, source, ac, _, releases := newManualTabSession(t, 1) | 301 | d, source, ac, _, releases := newManualTabSession(t, 1) | ||
| 284 | defer releaseAll(releases) | 302 | defer releaseAll(releases) | ||
| @@ -301,6 +319,11 @@ func TestParkedRouteLeaseExpiryClosesExactRetainedTransport(t *testing.T) { | |||||
| 301 | case <-time.After(time.Second): | 319 | case <-time.After(time.Second): | ||
| 302 | t.Fatal("parked transport remained open after its lease expired") | 320 | t.Fatal("parked transport remained open after its lease expired") | ||
| 303 | } | 321 | } | ||
| 322 | + | ac.parkedRouteMu.Lock() | |||
| 323 | + | require.Nil(t, ac.parkedRoute) | |||
| 324 | + | ac.parkedRouteMu.Unlock() | |||
| 325 | + | require.False(t, ac.parkedRouteOutput.Load()) | |||
| 326 | + | require.False(t, ac.parkedRouteFullPending.Load()) | |||
| 304 | } | 327 | } | ||
| 305 | 328 | ||||
| 306 | func TestParkedRouteMalformedSwitchRequestsFailClosed(t *testing.T) { | 329 | func TestParkedRouteMalformedSwitchRequestsFailClosed(t *testing.T) { | ||