fix(daemon): manage timers outside coordinator lock
- commit
- 25059b865503ec24741158e1542397a6c3ac42ed
- author
- brice · · 2026-07-11 19:51 UTC
- committer
- brice · · 2026-07-11 19:52 UTC
- parent
- 9c054e2
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
1 file · +106 -61
M internal/usecase/daemon/render_coordinator.go +106 -61
| @@ -145,7 +145,9 @@ type renderCoordinator struct { | |||
| 145 | 145 | // attachment is the currently bound client identity; callbacks from any | |
| 146 | 146 | // other identity are stale and must not mutate coordinator state. | |
| 147 | 147 | attachment *attachedClient | |
| 148 | - | torndown bool | |
| 148 | + | // detached rejects invalidations after detach/park until a new attachment. | |
| 149 | + | detached bool | |
| 150 | + | torndown bool | |
| 149 | 151 | ||
| 150 | 152 | resize resizeRequestMetadata | |
| 151 | 153 | metrics renderCoordinatorBurstMetrics | |
| @@ -173,17 +175,22 @@ func newRenderCoordinator(opts renderCoordinatorOptions) *renderCoordinator { | |||
| 173 | 175 | return &renderCoordinator{opts: opts} | |
| 174 | 176 | } | |
| 175 | 177 | ||
| 176 | - | // cancelNormalTimerLocked and cancelSyncTimerLocked release timer workers even | |
| 177 | - | // when a test clock supplies an inert channel. Caller holds c.mu. | |
| 178 | - | func (c *renderCoordinator) cancelNormalTimerLocked() { | |
| 179 | - | if c.normalTimer != nil { | |
| 180 | - | c.normalTimer.Stop() | |
| 181 | - | c.normalTimer = nil | |
| 182 | - | } | |
| 178 | + | // detachNormalTimerLocked releases coordinator ownership. The caller must stop | |
| 179 | + | // the returned timer after dropping c.mu: Timer methods are external callbacks. | |
| 180 | + | func (c *renderCoordinator) detachNormalTimerLocked() ports.Timer { | |
| 181 | + | timer := c.normalTimer | |
| 182 | + | c.normalTimer = nil | |
| 183 | 183 | if c.normalCancel != nil { | |
| 184 | 184 | close(c.normalCancel) | |
| 185 | 185 | c.normalCancel = nil | |
| 186 | 186 | } | |
| 187 | + | return timer | |
| 188 | + | } | |
| 189 | + | ||
| 190 | + | func stopTimer(timer ports.Timer) { | |
| 191 | + | if timer != nil { | |
| 192 | + | timer.Stop() | |
| 193 | + | } | |
| 187 | 194 | } | |
| 188 | 195 | ||
| 189 | 196 | type syncBatch struct { | |
| @@ -197,23 +204,30 @@ type syncBatch struct { | |||
| 197 | 204 | done chan struct{} | |
| 198 | 205 | } | |
| 199 | 206 | ||
| 200 | - | func cancelSyncBatchLocked(batch *syncBatch) { | |
| 201 | - | if batch.timer != nil { | |
| 202 | - | batch.timer.Stop() | |
| 203 | - | batch.timer = nil | |
| 204 | - | } | |
| 207 | + | func detachSyncBatchLocked(batch *syncBatch) ports.Timer { | |
| 208 | + | timer := batch.timer | |
| 209 | + | batch.timer = nil | |
| 205 | 210 | if batch.cancel != nil { | |
| 206 | 211 | close(batch.cancel) | |
| 207 | 212 | batch.cancel = nil | |
| 208 | 213 | } | |
| 214 | + | return timer | |
| 209 | 215 | } | |
| 210 | 216 | ||
| 211 | - | func (c *renderCoordinator) cancelSyncTimersLocked() { | |
| 217 | + | func (c *renderCoordinator) detachSyncTimersLocked() []ports.Timer { | |
| 218 | + | timers := make([]ports.Timer, 0, len(c.syncBatches)) | |
| 212 | 219 | for pane, batch := range c.syncBatches { | |
| 213 | - | cancelSyncBatchLocked(batch) | |
| 220 | + | timers = append(timers, detachSyncBatchLocked(batch)) | |
| 214 | 221 | delete(c.syncBatches, pane) | |
| 215 | 222 | c.syncRegistryVersion++ | |
| 216 | 223 | } | |
| 224 | + | return timers | |
| 225 | + | } | |
| 226 | + | ||
| 227 | + | func stopTimers(timers []ports.Timer) { | |
| 228 | + | for _, timer := range timers { | |
| 229 | + | stopTimer(timer) | |
| 230 | + | } | |
| 217 | 231 | } | |
| 218 | 232 | ||
| 219 | 233 | // invalidate publishes one producer state transition. Consecutive | |
| @@ -221,7 +235,7 @@ func (c *renderCoordinator) cancelSyncTimersLocked() { | |||
| 221 | 235 | // the stickiest reset win. | |
| 222 | 236 | func (c *renderCoordinator) invalidate(inv renderInvalidation) { | |
| 223 | 237 | c.mu.Lock() | |
| 224 | - | if c.torndown { | |
| 238 | + | if c.torndown || c.detached { | |
| 225 | 239 | c.mu.Unlock() | |
| 226 | 240 | return | |
| 227 | 241 | } | |
| @@ -237,49 +251,48 @@ func (c *renderCoordinator) invalidate(inv renderInvalidation) { | |||
| 237 | 251 | c.pendingUrgent = c.pendingUrgent || inv.class == invalidateUrgent | |
| 238 | 252 | c.pendingPreview = c.pendingPreview || c.previewWake != nil || len(c.previewWakes) != 0 | |
| 239 | 253 | c.coalesced++ | |
| 240 | - | // Cap one normal deadline for each primary or preview batch. Once a | |
| 241 | - | // preview has been delivered while the target is ACK-blocked, a later | |
| 242 | - | // target transition needs its own deadline even though the primary remains | |
| 243 | - | // pending. | |
| 244 | 254 | arm := !wasPending || (!wasUrgent && c.pendingUrgent) || (!wasPreviewPending && c.pendingPreview) | |
| 255 | + | var old ports.Timer | |
| 245 | 256 | if arm { | |
| 246 | 257 | c.generation++ | |
| 247 | - | c.cancelNormalTimerLocked() | |
| 258 | + | old = c.detachNormalTimerLocked() | |
| 248 | 259 | } | |
| 249 | 260 | gen := c.generation | |
| 250 | 261 | delay := minOutputRenderDeadline + time.Duration(c.outputPressure)*time.Millisecond | |
| 251 | 262 | if c.pendingUrgent { | |
| 252 | - | // Urgent state always supersedes bulk pressure and is never extended. | |
| 253 | 263 | delay = urgentRenderDeadline | |
| 254 | 264 | } | |
| 255 | 265 | clock := c.opts.clock | |
| 256 | 266 | c.armed = c.armed || arm | |
| 267 | + | c.mu.Unlock() | |
| 268 | + | stopTimer(old) | |
| 269 | + | if onInvalidate != nil { | |
| 270 | + | onInvalidate(inv) | |
| 271 | + | } | |
| 257 | 272 | if !arm || clock == nil { | |
| 258 | - | c.mu.Unlock() | |
| 259 | - | if onInvalidate != nil { | |
| 260 | - | onInvalidate(inv) | |
| 261 | - | } | |
| 262 | 273 | return | |
| 263 | 274 | } | |
| 275 | + | ||
| 276 | + | // NewTimer and C may re-enter the coordinator. Publish only if this | |
| 277 | + | // generation is still the reserved deadline after those external calls. | |
| 264 | 278 | timer := clock.NewTimer(delay) | |
| 265 | 279 | timerC := timer.C() | |
| 266 | - | // A nil timer channel is inert, so it cannot need cancellation. Complete | |
| 267 | - | // synchronously before allocating a worker or its coordination channels. | |
| 268 | - | if timerC == nil { | |
| 269 | - | c.mu.Unlock() | |
| 270 | - | if onInvalidate != nil { | |
| 271 | - | onInvalidate(inv) | |
| 272 | - | } | |
| 273 | - | timer.Stop() | |
| 274 | - | c.fire(gen, false, true) | |
| 275 | - | return | |
| 276 | - | } | |
| 277 | 280 | cancel := make(chan struct{}) | |
| 278 | 281 | done := make(chan struct{}) | |
| 279 | - | c.normalTimer, c.normalCancel, c.normalWorkerDone = timer, cancel, done | |
| 282 | + | c.mu.Lock() | |
| 283 | + | valid := !c.torndown && c.pending && c.generation == gen && c.normalTimer == nil | |
| 284 | + | if valid && timerC != nil { | |
| 285 | + | c.normalTimer, c.normalCancel, c.normalWorkerDone = timer, cancel, done | |
| 286 | + | } | |
| 280 | 287 | c.mu.Unlock() | |
| 281 | - | if onInvalidate != nil { | |
| 282 | - | onInvalidate(inv) | |
| 288 | + | if !valid { | |
| 289 | + | stopTimer(timer) | |
| 290 | + | return | |
| 291 | + | } | |
| 292 | + | if timerC == nil { | |
| 293 | + | stopTimer(timer) | |
| 294 | + | c.fire(gen, false, true) | |
| 295 | + | return | |
| 283 | 296 | } | |
| 284 | 297 | go func() { | |
| 285 | 298 | defer close(done) | |
| @@ -290,8 +303,6 @@ func (c *renderCoordinator) invalidate(inv renderInvalidation) { | |||
| 290 | 303 | case <-cancel: | |
| 291 | 304 | } | |
| 292 | 305 | }() | |
| 293 | - | // Give fake-clock callbacks a chance to block on C before the test | |
| 294 | - | // advances it; real clocks are unaffected. | |
| 295 | 306 | runtime.Gosched() | |
| 296 | 307 | } | |
| 297 | 308 | ||
| @@ -340,8 +351,9 @@ func (c *renderCoordinator) noteSyncBeginWithRenderability(p *pane, gen uint64, | |||
| 340 | 351 | if c.syncBatches == nil { | |
| 341 | 352 | c.syncBatches = make(map[*pane]*syncBatch) | |
| 342 | 353 | } | |
| 343 | - | if old := c.syncBatches[p]; old != nil { | |
| 344 | - | cancelSyncBatchLocked(old) | |
| 354 | + | var old ports.Timer | |
| 355 | + | if previous := c.syncBatches[p]; previous != nil { | |
| 356 | + | old = detachSyncBatchLocked(previous) | |
| 345 | 357 | } | |
| 346 | 358 | batch := &syncBatch{generation: gen, renderable: renderable} | |
| 347 | 359 | if len(force) != 0 { | |
| @@ -350,19 +362,32 @@ func (c *renderCoordinator) noteSyncBeginWithRenderability(p *pane, gen uint64, | |||
| 350 | 362 | c.syncBatches[p] = batch | |
| 351 | 363 | c.syncRegistryVersion++ | |
| 352 | 364 | clock := c.opts.clock | |
| 365 | + | c.mu.Unlock() | |
| 366 | + | stopTimer(old) | |
| 353 | 367 | if clock == nil { | |
| 354 | - | c.mu.Unlock() | |
| 355 | 368 | return | |
| 356 | 369 | } | |
| 357 | - | batch.timer = clock.NewTimer(maxSyncUpdateDuration) | |
| 358 | - | batch.cancel = make(chan struct{}) | |
| 359 | - | batch.done = make(chan struct{}) | |
| 360 | - | timer, cancel, done := batch.timer, batch.cancel, batch.done | |
| 370 | + | ||
| 371 | + | // Both timer operations are external. A reentrant callback may replace or | |
| 372 | + | // remove batch; identity validation below rejects this stale timer. | |
| 373 | + | timer := clock.NewTimer(maxSyncUpdateDuration) | |
| 374 | + | timerC := timer.C() | |
| 375 | + | cancel := make(chan struct{}) | |
| 376 | + | done := make(chan struct{}) | |
| 377 | + | c.mu.Lock() | |
| 378 | + | valid := !c.torndown && c.syncBatches[p] == batch && batch.generation == gen | |
| 379 | + | if valid { | |
| 380 | + | batch.timer, batch.cancel, batch.done = timer, cancel, done | |
| 381 | + | } | |
| 361 | 382 | c.mu.Unlock() | |
| 383 | + | if !valid { | |
| 384 | + | stopTimer(timer) | |
| 385 | + | return | |
| 386 | + | } | |
| 362 | 387 | go func() { | |
| 363 | 388 | defer close(done) | |
| 364 | 389 | select { | |
| 365 | - | case <-timer.C(): | |
| 390 | + | case <-timerC: | |
| 366 | 391 | case <-cancel: | |
| 367 | 392 | return | |
| 368 | 393 | } | |
| @@ -386,16 +411,18 @@ func (c *renderCoordinator) noteSyncBeginWithRenderability(p *pane, gen uint64, | |||
| 386 | 411 | c.mu.Lock() | |
| 387 | 412 | current = c.syncBatches[p] | |
| 388 | 413 | valid = !c.torndown && current == batch && current.generation == gen | |
| 414 | + | var stopped ports.Timer | |
| 389 | 415 | if valid { | |
| 390 | 416 | delete(c.syncBatches, p) | |
| 391 | 417 | c.syncRegistryVersion++ | |
| 392 | - | cancelSyncBatchLocked(batch) | |
| 418 | + | stopped = detachSyncBatchLocked(batch) | |
| 393 | 419 | if len(c.syncBatches) == 0 && c.pending { | |
| 394 | 420 | c.pendingUrgent = true | |
| 395 | 421 | } | |
| 396 | 422 | } | |
| 397 | 423 | c.mu.Unlock() | |
| 398 | 424 | if valid { | |
| 425 | + | stopTimer(stopped) | |
| 399 | 426 | // Reevaluate the aggregate gate only after forcing this pane. | |
| 400 | 427 | c.fireCurrent(true) | |
| 401 | 428 | } | |
| @@ -426,29 +453,33 @@ func (c *renderCoordinator) noteSyncEnd(p *pane, gen uint64) { | |||
| 426 | 453 | // The caller must invoke fireCurrent only after releasing pane.mu. | |
| 427 | 454 | func (c *renderCoordinator) removeSyncEnd(p *pane, gen uint64, renderable bool) bool { | |
| 428 | 455 | c.mu.Lock() | |
| 429 | - | defer c.mu.Unlock() | |
| 430 | 456 | batch := c.syncBatches[p] | |
| 431 | 457 | if batch == nil || batch.generation != gen { | |
| 458 | + | c.mu.Unlock() | |
| 432 | 459 | return false | |
| 433 | 460 | } | |
| 434 | 461 | delete(c.syncBatches, p) | |
| 435 | 462 | c.syncRegistryVersion++ | |
| 436 | - | cancelSyncBatchLocked(batch) | |
| 463 | + | timer := detachSyncBatchLocked(batch) | |
| 437 | 464 | if renderable && len(c.syncBatches) == 0 && c.pending { | |
| 438 | 465 | c.pendingUrgent = true | |
| 439 | 466 | } | |
| 467 | + | c.mu.Unlock() | |
| 468 | + | stopTimer(timer) | |
| 440 | 469 | return renderable | |
| 441 | 470 | } | |
| 442 | 471 | ||
| 443 | 472 | // noteSyncPaneRemoved releases a pane watchdog when pane lifecycle ends. | |
| 444 | 473 | func (c *renderCoordinator) noteSyncPaneRemoved(p *pane) { | |
| 445 | 474 | c.mu.Lock() | |
| 475 | + | var timer ports.Timer | |
| 446 | 476 | if batch := c.syncBatches[p]; batch != nil { | |
| 447 | 477 | delete(c.syncBatches, p) | |
| 448 | 478 | c.syncRegistryVersion++ | |
| 449 | - | cancelSyncBatchLocked(batch) | |
| 479 | + | timer = detachSyncBatchLocked(batch) | |
| 450 | 480 | } | |
| 451 | 481 | c.mu.Unlock() | |
| 482 | + | stopTimer(timer) | |
| 452 | 483 | } | |
| 453 | 484 | ||
| 454 | 485 | // subscribePreview installs fn as the preview observer for coalesced wakes. | |
| @@ -489,6 +520,7 @@ func (c *renderCoordinator) attach(ac *attachedClient) { | |||
| 489 | 520 | c.mu.Lock() | |
| 490 | 521 | if !c.torndown { | |
| 491 | 522 | c.attachment = ac | |
| 523 | + | c.detached = false | |
| 492 | 524 | } | |
| 493 | 525 | c.mu.Unlock() | |
| 494 | 526 | } | |
| @@ -497,17 +529,22 @@ func (c *renderCoordinator) attach(ac *attachedClient) { | |||
| 497 | 529 | // attachment. | |
| 498 | 530 | func (c *renderCoordinator) noteDetach(ac *attachedClient) { | |
| 499 | 531 | c.mu.Lock() | |
| 532 | + | var timer ports.Timer | |
| 533 | + | var timers []ports.Timer | |
| 500 | 534 | if c.attachment == ac { | |
| 501 | 535 | c.attachment = nil | |
| 536 | + | c.detached = true | |
| 502 | 537 | c.pending = false | |
| 503 | 538 | c.ackDeferred = false | |
| 504 | 539 | c.pendingPreview = false | |
| 505 | 540 | c.generation++ | |
| 506 | 541 | c.armed = false | |
| 507 | - | c.cancelNormalTimerLocked() | |
| 508 | - | c.cancelSyncTimersLocked() | |
| 542 | + | timer = c.detachNormalTimerLocked() | |
| 543 | + | timers = c.detachSyncTimersLocked() | |
| 509 | 544 | } | |
| 510 | 545 | c.mu.Unlock() | |
| 546 | + | stopTimer(timer) | |
| 547 | + | stopTimers(timers) | |
| 511 | 548 | } | |
| 512 | 549 | ||
| 513 | 550 | // noteReplace hands the coordinator from old to replacement; callbacks | |
| @@ -518,17 +555,22 @@ func (c *renderCoordinator) noteReplace(old, replacement *attachedClient) { | |||
| 518 | 555 | // which predated coordinator ownership. In that case nil has no pending | |
| 519 | 556 | // identity to invalidate, and the replacement becomes the first bound | |
| 520 | 557 | // attachment atomically with this lifecycle transition. | |
| 558 | + | var timer ports.Timer | |
| 559 | + | var timers []ports.Timer | |
| 521 | 560 | if c.attachment == old || c.attachment == nil { | |
| 522 | 561 | c.attachment = replacement | |
| 562 | + | c.detached = false | |
| 523 | 563 | c.pending = false | |
| 524 | 564 | c.ackDeferred = false | |
| 525 | 565 | c.pendingPreview = false | |
| 526 | 566 | c.generation++ | |
| 527 | 567 | c.armed = false | |
| 528 | - | c.cancelNormalTimerLocked() | |
| 529 | - | c.cancelSyncTimersLocked() | |
| 568 | + | timer = c.detachNormalTimerLocked() | |
| 569 | + | timers = c.detachSyncTimersLocked() | |
| 530 | 570 | } | |
| 531 | 571 | c.mu.Unlock() | |
| 572 | + | stopTimer(timer) | |
| 573 | + | stopTimers(timers) | |
| 532 | 574 | } | |
| 533 | 575 | ||
| 534 | 576 | // notePark invalidates pending wakes when the attachment parks for resume. | |
| @@ -546,9 +588,11 @@ func (c *renderCoordinator) noteSessionTeardown() { | |||
| 546 | 588 | c.pendingPreview = false | |
| 547 | 589 | c.generation++ | |
| 548 | 590 | c.armed = false | |
| 549 | - | c.cancelNormalTimerLocked() | |
| 550 | - | c.cancelSyncTimersLocked() | |
| 591 | + | timer := c.detachNormalTimerLocked() | |
| 592 | + | timers := c.detachSyncTimersLocked() | |
| 551 | 593 | c.mu.Unlock() | |
| 594 | + | stopTimer(timer) | |
| 595 | + | stopTimers(timers) | |
| 552 | 596 | } | |
| 553 | 597 | ||
| 554 | 598 | // recordResizeRequest records the latest requested geometry and source before | |
| @@ -678,10 +722,11 @@ func (c *renderCoordinator) fire(gen uint64, watchdog, deadline bool) { | |||
| 678 | 722 | c.outputPressure-- | |
| 679 | 723 | } | |
| 680 | 724 | } | |
| 681 | - | c.cancelNormalTimerLocked() | |
| 725 | + | timer := c.detachNormalTimerLocked() | |
| 682 | 726 | c.pending, c.pendingReset, c.pendingUrgent, c.ackDeferred, c.deadlineDue, c.coalesced, c.armed = false, false, false, false, false, 0, false | |
| 683 | 727 | wake := c.opts.wake | |
| 684 | 728 | c.mu.Unlock() | |
| 729 | + | stopTimer(timer) | |
| 685 | 730 | if wake != nil { | |
| 686 | 731 | wake(w) | |
| 687 | 732 | } | |
| @@ -145,7 +145,9 @@ type renderCoordinator struct { | |||||
| 145 | // attachment is the currently bound client identity; callbacks from any | 145 | // attachment is the currently bound client identity; callbacks from any | ||
| 146 | // other identity are stale and must not mutate coordinator state. | 146 | // other identity are stale and must not mutate coordinator state. | ||
| 147 | attachment *attachedClient | 147 | attachment *attachedClient | ||
| 148 | - | torndown bool | 148 | + | // detached rejects invalidations after detach/park until a new attachment. |
| 149 | + | detached bool | |||
| 150 | + | torndown bool | |||
| 149 | 151 | ||||
| 150 | resize resizeRequestMetadata | 152 | resize resizeRequestMetadata | ||
| 151 | metrics renderCoordinatorBurstMetrics | 153 | metrics renderCoordinatorBurstMetrics | ||
| @@ -173,17 +175,22 @@ func newRenderCoordinator(opts renderCoordinatorOptions) *renderCoordinator { | |||||
| 173 | return &renderCoordinator{opts: opts} | 175 | return &renderCoordinator{opts: opts} | ||
| 174 | } | 176 | } | ||
| 175 | 177 | ||||
| 176 | - | // cancelNormalTimerLocked and cancelSyncTimerLocked release timer workers even | 178 | + | // detachNormalTimerLocked releases coordinator ownership. The caller must stop |
| 177 | - | // when a test clock supplies an inert channel. Caller holds c.mu. | 179 | + | // the returned timer after dropping c.mu: Timer methods are external callbacks. |
| 178 | - | func (c *renderCoordinator) cancelNormalTimerLocked() { | 180 | + | func (c *renderCoordinator) detachNormalTimerLocked() ports.Timer { |
| 179 | - | if c.normalTimer != nil { | 181 | + | timer := c.normalTimer |
| 180 | - | c.normalTimer.Stop() | 182 | + | c.normalTimer = nil |
| 181 | - | c.normalTimer = nil | |||
| 182 | - | } | |||
| 183 | if c.normalCancel != nil { | 183 | if c.normalCancel != nil { | ||
| 184 | close(c.normalCancel) | 184 | close(c.normalCancel) | ||
| 185 | c.normalCancel = nil | 185 | c.normalCancel = nil | ||
| 186 | } | 186 | } | ||
| 187 | + | return timer | |||
| 188 | + | } | |||
| 189 | + | ||||
| 190 | + | func stopTimer(timer ports.Timer) { | |||
| 191 | + | if timer != nil { | |||
| 192 | + | timer.Stop() | |||
| 193 | + | } | |||
| 187 | } | 194 | } | ||
| 188 | 195 | ||||
| 189 | type syncBatch struct { | 196 | type syncBatch struct { | ||
| @@ -197,23 +204,30 @@ type syncBatch struct { | |||||
| 197 | done chan struct{} | 204 | done chan struct{} | ||
| 198 | } | 205 | } | ||
| 199 | 206 | ||||
| 200 | - | func cancelSyncBatchLocked(batch *syncBatch) { | 207 | + | func detachSyncBatchLocked(batch *syncBatch) ports.Timer { |
| 201 | - | if batch.timer != nil { | 208 | + | timer := batch.timer |
| 202 | - | batch.timer.Stop() | 209 | + | batch.timer = nil |
| 203 | - | batch.timer = nil | |||
| 204 | - | } | |||
| 205 | if batch.cancel != nil { | 210 | if batch.cancel != nil { | ||
| 206 | close(batch.cancel) | 211 | close(batch.cancel) | ||
| 207 | batch.cancel = nil | 212 | batch.cancel = nil | ||
| 208 | } | 213 | } | ||
| 214 | + | return timer | |||
| 209 | } | 215 | } | ||
| 210 | 216 | ||||
| 211 | - | func (c *renderCoordinator) cancelSyncTimersLocked() { | 217 | + | func (c *renderCoordinator) detachSyncTimersLocked() []ports.Timer { |
| 218 | + | timers := make([]ports.Timer, 0, len(c.syncBatches)) | |||
| 212 | for pane, batch := range c.syncBatches { | 219 | for pane, batch := range c.syncBatches { | ||
| 213 | - | cancelSyncBatchLocked(batch) | 220 | + | timers = append(timers, detachSyncBatchLocked(batch)) |
| 214 | delete(c.syncBatches, pane) | 221 | delete(c.syncBatches, pane) | ||
| 215 | c.syncRegistryVersion++ | 222 | c.syncRegistryVersion++ | ||
| 216 | } | 223 | } | ||
| 224 | + | return timers | |||
| 225 | + | } | |||
| 226 | + | ||||
| 227 | + | func stopTimers(timers []ports.Timer) { | |||
| 228 | + | for _, timer := range timers { | |||
| 229 | + | stopTimer(timer) | |||
| 230 | + | } | |||
| 217 | } | 231 | } | ||
| 218 | 232 | ||||
| 219 | // invalidate publishes one producer state transition. Consecutive | 233 | // invalidate publishes one producer state transition. Consecutive | ||
| @@ -221,7 +235,7 @@ func (c *renderCoordinator) cancelSyncTimersLocked() { | |||||
| 221 | // the stickiest reset win. | 235 | // the stickiest reset win. | ||
| 222 | func (c *renderCoordinator) invalidate(inv renderInvalidation) { | 236 | func (c *renderCoordinator) invalidate(inv renderInvalidation) { | ||
| 223 | c.mu.Lock() | 237 | c.mu.Lock() | ||
| 224 | - | if c.torndown { | 238 | + | if c.torndown || c.detached { |
| 225 | c.mu.Unlock() | 239 | c.mu.Unlock() | ||
| 226 | return | 240 | return | ||
| 227 | } | 241 | } | ||
| @@ -237,49 +251,48 @@ func (c *renderCoordinator) invalidate(inv renderInvalidation) { | |||||
| 237 | c.pendingUrgent = c.pendingUrgent || inv.class == invalidateUrgent | 251 | c.pendingUrgent = c.pendingUrgent || inv.class == invalidateUrgent | ||
| 238 | c.pendingPreview = c.pendingPreview || c.previewWake != nil || len(c.previewWakes) != 0 | 252 | c.pendingPreview = c.pendingPreview || c.previewWake != nil || len(c.previewWakes) != 0 | ||
| 239 | c.coalesced++ | 253 | c.coalesced++ | ||
| 240 | - | // Cap one normal deadline for each primary or preview batch. Once a | |||
| 241 | - | // preview has been delivered while the target is ACK-blocked, a later | |||
| 242 | - | // target transition needs its own deadline even though the primary remains | |||
| 243 | - | // pending. | |||
| 244 | arm := !wasPending || (!wasUrgent && c.pendingUrgent) || (!wasPreviewPending && c.pendingPreview) | 254 | arm := !wasPending || (!wasUrgent && c.pendingUrgent) || (!wasPreviewPending && c.pendingPreview) | ||
| 255 | + | var old ports.Timer | |||
| 245 | if arm { | 256 | if arm { | ||
| 246 | c.generation++ | 257 | c.generation++ | ||
| 247 | - | c.cancelNormalTimerLocked() | 258 | + | old = c.detachNormalTimerLocked() |
| 248 | } | 259 | } | ||
| 249 | gen := c.generation | 260 | gen := c.generation | ||
| 250 | delay := minOutputRenderDeadline + time.Duration(c.outputPressure)*time.Millisecond | 261 | delay := minOutputRenderDeadline + time.Duration(c.outputPressure)*time.Millisecond | ||
| 251 | if c.pendingUrgent { | 262 | if c.pendingUrgent { | ||
| 252 | - | // Urgent state always supersedes bulk pressure and is never extended. | |||
| 253 | delay = urgentRenderDeadline | 263 | delay = urgentRenderDeadline | ||
| 254 | } | 264 | } | ||
| 255 | clock := c.opts.clock | 265 | clock := c.opts.clock | ||
| 256 | c.armed = c.armed || arm | 266 | c.armed = c.armed || arm | ||
| 267 | + | c.mu.Unlock() | |||
| 268 | + | stopTimer(old) | |||
| 269 | + | if onInvalidate != nil { | |||
| 270 | + | onInvalidate(inv) | |||
| 271 | + | } | |||
| 257 | if !arm || clock == nil { | 272 | if !arm || clock == nil { | ||
| 258 | - | c.mu.Unlock() | |||
| 259 | - | if onInvalidate != nil { | |||
| 260 | - | onInvalidate(inv) | |||
| 261 | - | } | |||
| 262 | return | 273 | return | ||
| 263 | } | 274 | } | ||
| 275 | + | ||||
| 276 | + | // NewTimer and C may re-enter the coordinator. Publish only if this | |||
| 277 | + | // generation is still the reserved deadline after those external calls. | |||
| 264 | timer := clock.NewTimer(delay) | 278 | timer := clock.NewTimer(delay) | ||
| 265 | timerC := timer.C() | 279 | timerC := timer.C() | ||
| 266 | - | // A nil timer channel is inert, so it cannot need cancellation. Complete | |||
| 267 | - | // synchronously before allocating a worker or its coordination channels. | |||
| 268 | - | if timerC == nil { | |||
| 269 | - | c.mu.Unlock() | |||
| 270 | - | if onInvalidate != nil { | |||
| 271 | - | onInvalidate(inv) | |||
| 272 | - | } | |||
| 273 | - | timer.Stop() | |||
| 274 | - | c.fire(gen, false, true) | |||
| 275 | - | return | |||
| 276 | - | } | |||
| 277 | cancel := make(chan struct{}) | 280 | cancel := make(chan struct{}) | ||
| 278 | done := make(chan struct{}) | 281 | done := make(chan struct{}) | ||
| 279 | - | c.normalTimer, c.normalCancel, c.normalWorkerDone = timer, cancel, done | 282 | + | c.mu.Lock() |
| 283 | + | valid := !c.torndown && c.pending && c.generation == gen && c.normalTimer == nil | |||
| 284 | + | if valid && timerC != nil { | |||
| 285 | + | c.normalTimer, c.normalCancel, c.normalWorkerDone = timer, cancel, done | |||
| 286 | + | } | |||
| 280 | c.mu.Unlock() | 287 | c.mu.Unlock() | ||
| 281 | - | if onInvalidate != nil { | 288 | + | if !valid { |
| 282 | - | onInvalidate(inv) | 289 | + | stopTimer(timer) |
| 290 | + | return | |||
| 291 | + | } | |||
| 292 | + | if timerC == nil { | |||
| 293 | + | stopTimer(timer) | |||
| 294 | + | c.fire(gen, false, true) | |||
| 295 | + | return | |||
| 283 | } | 296 | } | ||
| 284 | go func() { | 297 | go func() { | ||
| 285 | defer close(done) | 298 | defer close(done) | ||
| @@ -290,8 +303,6 @@ func (c *renderCoordinator) invalidate(inv renderInvalidation) { | |||||
| 290 | case <-cancel: | 303 | case <-cancel: | ||
| 291 | } | 304 | } | ||
| 292 | }() | 305 | }() | ||
| 293 | - | // Give fake-clock callbacks a chance to block on C before the test | |||
| 294 | - | // advances it; real clocks are unaffected. | |||
| 295 | runtime.Gosched() | 306 | runtime.Gosched() | ||
| 296 | } | 307 | } | ||
| 297 | 308 | ||||
| @@ -340,8 +351,9 @@ func (c *renderCoordinator) noteSyncBeginWithRenderability(p *pane, gen uint64, | |||||
| 340 | if c.syncBatches == nil { | 351 | if c.syncBatches == nil { | ||
| 341 | c.syncBatches = make(map[*pane]*syncBatch) | 352 | c.syncBatches = make(map[*pane]*syncBatch) | ||
| 342 | } | 353 | } | ||
| 343 | - | if old := c.syncBatches[p]; old != nil { | 354 | + | var old ports.Timer |
| 344 | - | cancelSyncBatchLocked(old) | 355 | + | if previous := c.syncBatches[p]; previous != nil { |
| 356 | + | old = detachSyncBatchLocked(previous) | |||
| 345 | } | 357 | } | ||
| 346 | batch := &syncBatch{generation: gen, renderable: renderable} | 358 | batch := &syncBatch{generation: gen, renderable: renderable} | ||
| 347 | if len(force) != 0 { | 359 | if len(force) != 0 { | ||
| @@ -350,19 +362,32 @@ func (c *renderCoordinator) noteSyncBeginWithRenderability(p *pane, gen uint64, | |||||
| 350 | c.syncBatches[p] = batch | 362 | c.syncBatches[p] = batch | ||
| 351 | c.syncRegistryVersion++ | 363 | c.syncRegistryVersion++ | ||
| 352 | clock := c.opts.clock | 364 | clock := c.opts.clock | ||
| 365 | + | c.mu.Unlock() | |||
| 366 | + | stopTimer(old) | |||
| 353 | if clock == nil { | 367 | if clock == nil { | ||
| 354 | - | c.mu.Unlock() | |||
| 355 | return | 368 | return | ||
| 356 | } | 369 | } | ||
| 357 | - | batch.timer = clock.NewTimer(maxSyncUpdateDuration) | 370 | + | |
| 358 | - | batch.cancel = make(chan struct{}) | 371 | + | // Both timer operations are external. A reentrant callback may replace or |
| 359 | - | batch.done = make(chan struct{}) | 372 | + | // remove batch; identity validation below rejects this stale timer. |
| 360 | - | timer, cancel, done := batch.timer, batch.cancel, batch.done | 373 | + | timer := clock.NewTimer(maxSyncUpdateDuration) |
| 374 | + | timerC := timer.C() | |||
| 375 | + | cancel := make(chan struct{}) | |||
| 376 | + | done := make(chan struct{}) | |||
| 377 | + | c.mu.Lock() | |||
| 378 | + | valid := !c.torndown && c.syncBatches[p] == batch && batch.generation == gen | |||
| 379 | + | if valid { | |||
| 380 | + | batch.timer, batch.cancel, batch.done = timer, cancel, done | |||
| 381 | + | } | |||
| 361 | c.mu.Unlock() | 382 | c.mu.Unlock() | ||
| 383 | + | if !valid { | |||
| 384 | + | stopTimer(timer) | |||
| 385 | + | return | |||
| 386 | + | } | |||
| 362 | go func() { | 387 | go func() { | ||
| 363 | defer close(done) | 388 | defer close(done) | ||
| 364 | select { | 389 | select { | ||
| 365 | - | case <-timer.C(): | 390 | + | case <-timerC: |
| 366 | case <-cancel: | 391 | case <-cancel: | ||
| 367 | return | 392 | return | ||
| 368 | } | 393 | } | ||
| @@ -386,16 +411,18 @@ func (c *renderCoordinator) noteSyncBeginWithRenderability(p *pane, gen uint64, | |||||
| 386 | c.mu.Lock() | 411 | c.mu.Lock() | ||
| 387 | current = c.syncBatches[p] | 412 | current = c.syncBatches[p] | ||
| 388 | valid = !c.torndown && current == batch && current.generation == gen | 413 | valid = !c.torndown && current == batch && current.generation == gen | ||
| 414 | + | var stopped ports.Timer | |||
| 389 | if valid { | 415 | if valid { | ||
| 390 | delete(c.syncBatches, p) | 416 | delete(c.syncBatches, p) | ||
| 391 | c.syncRegistryVersion++ | 417 | c.syncRegistryVersion++ | ||
| 392 | - | cancelSyncBatchLocked(batch) | 418 | + | stopped = detachSyncBatchLocked(batch) |
| 393 | if len(c.syncBatches) == 0 && c.pending { | 419 | if len(c.syncBatches) == 0 && c.pending { | ||
| 394 | c.pendingUrgent = true | 420 | c.pendingUrgent = true | ||
| 395 | } | 421 | } | ||
| 396 | } | 422 | } | ||
| 397 | c.mu.Unlock() | 423 | c.mu.Unlock() | ||
| 398 | if valid { | 424 | if valid { | ||
| 425 | + | stopTimer(stopped) | |||
| 399 | // Reevaluate the aggregate gate only after forcing this pane. | 426 | // Reevaluate the aggregate gate only after forcing this pane. | ||
| 400 | c.fireCurrent(true) | 427 | c.fireCurrent(true) | ||
| 401 | } | 428 | } | ||
| @@ -426,29 +453,33 @@ func (c *renderCoordinator) noteSyncEnd(p *pane, gen uint64) { | |||||
| 426 | // The caller must invoke fireCurrent only after releasing pane.mu. | 453 | // The caller must invoke fireCurrent only after releasing pane.mu. | ||
| 427 | func (c *renderCoordinator) removeSyncEnd(p *pane, gen uint64, renderable bool) bool { | 454 | func (c *renderCoordinator) removeSyncEnd(p *pane, gen uint64, renderable bool) bool { | ||
| 428 | c.mu.Lock() | 455 | c.mu.Lock() | ||
| 429 | - | defer c.mu.Unlock() | |||
| 430 | batch := c.syncBatches[p] | 456 | batch := c.syncBatches[p] | ||
| 431 | if batch == nil || batch.generation != gen { | 457 | if batch == nil || batch.generation != gen { | ||
| 458 | + | c.mu.Unlock() | |||
| 432 | return false | 459 | return false | ||
| 433 | } | 460 | } | ||
| 434 | delete(c.syncBatches, p) | 461 | delete(c.syncBatches, p) | ||
| 435 | c.syncRegistryVersion++ | 462 | c.syncRegistryVersion++ | ||
| 436 | - | cancelSyncBatchLocked(batch) | 463 | + | timer := detachSyncBatchLocked(batch) |
| 437 | if renderable && len(c.syncBatches) == 0 && c.pending { | 464 | if renderable && len(c.syncBatches) == 0 && c.pending { | ||
| 438 | c.pendingUrgent = true | 465 | c.pendingUrgent = true | ||
| 439 | } | 466 | } | ||
| 467 | + | c.mu.Unlock() | |||
| 468 | + | stopTimer(timer) | |||
| 440 | return renderable | 469 | return renderable | ||
| 441 | } | 470 | } | ||
| 442 | 471 | ||||
| 443 | // noteSyncPaneRemoved releases a pane watchdog when pane lifecycle ends. | 472 | // noteSyncPaneRemoved releases a pane watchdog when pane lifecycle ends. | ||
| 444 | func (c *renderCoordinator) noteSyncPaneRemoved(p *pane) { | 473 | func (c *renderCoordinator) noteSyncPaneRemoved(p *pane) { | ||
| 445 | c.mu.Lock() | 474 | c.mu.Lock() | ||
| 475 | + | var timer ports.Timer | |||
| 446 | if batch := c.syncBatches[p]; batch != nil { | 476 | if batch := c.syncBatches[p]; batch != nil { | ||
| 447 | delete(c.syncBatches, p) | 477 | delete(c.syncBatches, p) | ||
| 448 | c.syncRegistryVersion++ | 478 | c.syncRegistryVersion++ | ||
| 449 | - | cancelSyncBatchLocked(batch) | 479 | + | timer = detachSyncBatchLocked(batch) |
| 450 | } | 480 | } | ||
| 451 | c.mu.Unlock() | 481 | c.mu.Unlock() | ||
| 482 | + | stopTimer(timer) | |||
| 452 | } | 483 | } | ||
| 453 | 484 | ||||
| 454 | // subscribePreview installs fn as the preview observer for coalesced wakes. | 485 | // subscribePreview installs fn as the preview observer for coalesced wakes. | ||
| @@ -489,6 +520,7 @@ func (c *renderCoordinator) attach(ac *attachedClient) { | |||||
| 489 | c.mu.Lock() | 520 | c.mu.Lock() | ||
| 490 | if !c.torndown { | 521 | if !c.torndown { | ||
| 491 | c.attachment = ac | 522 | c.attachment = ac | ||
| 523 | + | c.detached = false | |||
| 492 | } | 524 | } | ||
| 493 | c.mu.Unlock() | 525 | c.mu.Unlock() | ||
| 494 | } | 526 | } | ||
| @@ -497,17 +529,22 @@ func (c *renderCoordinator) attach(ac *attachedClient) { | |||||
| 497 | // attachment. | 529 | // attachment. | ||
| 498 | func (c *renderCoordinator) noteDetach(ac *attachedClient) { | 530 | func (c *renderCoordinator) noteDetach(ac *attachedClient) { | ||
| 499 | c.mu.Lock() | 531 | c.mu.Lock() | ||
| 532 | + | var timer ports.Timer | |||
| 533 | + | var timers []ports.Timer | |||
| 500 | if c.attachment == ac { | 534 | if c.attachment == ac { | ||
| 501 | c.attachment = nil | 535 | c.attachment = nil | ||
| 536 | + | c.detached = true | |||
| 502 | c.pending = false | 537 | c.pending = false | ||
| 503 | c.ackDeferred = false | 538 | c.ackDeferred = false | ||
| 504 | c.pendingPreview = false | 539 | c.pendingPreview = false | ||
| 505 | c.generation++ | 540 | c.generation++ | ||
| 506 | c.armed = false | 541 | c.armed = false | ||
| 507 | - | c.cancelNormalTimerLocked() | 542 | + | timer = c.detachNormalTimerLocked() |
| 508 | - | c.cancelSyncTimersLocked() | 543 | + | timers = c.detachSyncTimersLocked() |
| 509 | } | 544 | } | ||
| 510 | c.mu.Unlock() | 545 | c.mu.Unlock() | ||
| 546 | + | stopTimer(timer) | |||
| 547 | + | stopTimers(timers) | |||
| 511 | } | 548 | } | ||
| 512 | 549 | ||||
| 513 | // noteReplace hands the coordinator from old to replacement; callbacks | 550 | // noteReplace hands the coordinator from old to replacement; callbacks | ||
| @@ -518,17 +555,22 @@ func (c *renderCoordinator) noteReplace(old, replacement *attachedClient) { | |||||
| 518 | // which predated coordinator ownership. In that case nil has no pending | 555 | // which predated coordinator ownership. In that case nil has no pending | ||
| 519 | // identity to invalidate, and the replacement becomes the first bound | 556 | // identity to invalidate, and the replacement becomes the first bound | ||
| 520 | // attachment atomically with this lifecycle transition. | 557 | // attachment atomically with this lifecycle transition. | ||
| 558 | + | var timer ports.Timer | |||
| 559 | + | var timers []ports.Timer | |||
| 521 | if c.attachment == old || c.attachment == nil { | 560 | if c.attachment == old || c.attachment == nil { | ||
| 522 | c.attachment = replacement | 561 | c.attachment = replacement | ||
| 562 | + | c.detached = false | |||
| 523 | c.pending = false | 563 | c.pending = false | ||
| 524 | c.ackDeferred = false | 564 | c.ackDeferred = false | ||
| 525 | c.pendingPreview = false | 565 | c.pendingPreview = false | ||
| 526 | c.generation++ | 566 | c.generation++ | ||
| 527 | c.armed = false | 567 | c.armed = false | ||
| 528 | - | c.cancelNormalTimerLocked() | 568 | + | timer = c.detachNormalTimerLocked() |
| 529 | - | c.cancelSyncTimersLocked() | 569 | + | timers = c.detachSyncTimersLocked() |
| 530 | } | 570 | } | ||
| 531 | c.mu.Unlock() | 571 | c.mu.Unlock() | ||
| 572 | + | stopTimer(timer) | |||
| 573 | + | stopTimers(timers) | |||
| 532 | } | 574 | } | ||
| 533 | 575 | ||||
| 534 | // notePark invalidates pending wakes when the attachment parks for resume. | 576 | // notePark invalidates pending wakes when the attachment parks for resume. | ||
| @@ -546,9 +588,11 @@ func (c *renderCoordinator) noteSessionTeardown() { | |||||
| 546 | c.pendingPreview = false | 588 | c.pendingPreview = false | ||
| 547 | c.generation++ | 589 | c.generation++ | ||
| 548 | c.armed = false | 590 | c.armed = false | ||
| 549 | - | c.cancelNormalTimerLocked() | 591 | + | timer := c.detachNormalTimerLocked() |
| 550 | - | c.cancelSyncTimersLocked() | 592 | + | timers := c.detachSyncTimersLocked() |
| 551 | c.mu.Unlock() | 593 | c.mu.Unlock() | ||
| 594 | + | stopTimer(timer) | |||
| 595 | + | stopTimers(timers) | |||
| 552 | } | 596 | } | ||
| 553 | 597 | ||||
| 554 | // recordResizeRequest records the latest requested geometry and source before | 598 | // recordResizeRequest records the latest requested geometry and source before | ||
| @@ -678,10 +722,11 @@ func (c *renderCoordinator) fire(gen uint64, watchdog, deadline bool) { | |||||
| 678 | c.outputPressure-- | 722 | c.outputPressure-- | ||
| 679 | } | 723 | } | ||
| 680 | } | 724 | } | ||
| 681 | - | c.cancelNormalTimerLocked() | 725 | + | timer := c.detachNormalTimerLocked() |
| 682 | c.pending, c.pendingReset, c.pendingUrgent, c.ackDeferred, c.deadlineDue, c.coalesced, c.armed = false, false, false, false, false, 0, false | 726 | c.pending, c.pendingReset, c.pendingUrgent, c.ackDeferred, c.deadlineDue, c.coalesced, c.armed = false, false, false, false, false, 0, false | ||
| 683 | wake := c.opts.wake | 727 | wake := c.opts.wake | ||
| 684 | c.mu.Unlock() | 728 | c.mu.Unlock() | ||
| 729 | + | stopTimer(timer) | |||
| 685 | if wake != nil { | 730 | if wake != nil { | ||
| 686 | wake(w) | 731 | wake(w) | ||
| 687 | } | 732 | } | ||