fix(daemon): read attachment lease readiness under the coordinator lock
fireWithTimerTokenAndLease read lease.ready after releasing c.mu while markAttachmentReady writes it under c.mu, a data race seen in CI. Leases are now filtered for readiness while the lock is held.
- commit
- dce3fad94ec4d4a2f19f4f9cd557794aa6694ca9
- author
- brice · · 2026-10-08 08:07 UTC
- parent
- f67cb47
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
1 file · +9 -9
M internal/usecase/daemon/render_coordinator.go +9 -9
| @@ -1005,21 +1005,21 @@ func (c *renderCoordinator) fireWithTimerTokenAndLease(token *timerToken, gen ui | |||
| 1005 | 1005 | if c.pendingGeneration == 0 { | |
| 1006 | 1006 | c.pendingGeneration = 1 | |
| 1007 | 1007 | } | |
| 1008 | - | leases := make(map[*attachedClient]*attachmentLease, len(c.leases)) | |
| 1008 | + | // lease.ready is guarded by c.mu (markAttachmentReady writes it), so only | |
| 1009 | + | // leases already ready under the lock are probed after unlocking. | |
| 1010 | + | probe := make(map[*attachedClient]*attachmentLease, len(c.leases)) | |
| 1009 | 1011 | for ac, lease := range c.leases { | |
| 1010 | - | if lease != nil && lease.active { | |
| 1011 | - | leases[ac] = lease | |
| 1012 | + | if lease != nil && lease.active && lease.ready { | |
| 1013 | + | probe[ac] = lease | |
| 1012 | 1014 | } | |
| 1013 | 1015 | } | |
| 1014 | 1016 | c.mu.Unlock() | |
| 1015 | 1017 | ||
| 1016 | - | ready := make(map[*attachmentLease]bool, len(leases)) | |
| 1017 | - | for ac, lease := range leases { | |
| 1018 | + | ready := make(map[*attachmentLease]bool, len(probe)) | |
| 1019 | + | for ac, lease := range probe { | |
| 1018 | 1020 | // Welcome holds attachment.sendMu while its transport Send is in | |
| 1019 | 1021 | // flight. Do not probe an incarnation before it is ready. | |
| 1020 | - | if lease.ready { | |
| 1021 | - | ready[lease] = c.attachmentAckReady(ac) | |
| 1022 | - | } | |
| 1022 | + | ready[lease] = c.attachmentAckReady(ac) | |
| 1023 | 1023 | } | |
| 1024 | 1024 | ||
| 1025 | 1025 | // The readiness callback can detach, replace, or publish a sync batch. | |
| @@ -1041,7 +1041,7 @@ func (c *renderCoordinator) fireWithTimerTokenAndLease(token *timerToken, gen ui | |||
| 1041 | 1041 | ||
| 1042 | 1042 | // Re-snapshot after every external readiness probe. A replacement may have | |
| 1043 | 1043 | // retired one of the leases we probed while the callback was unlocked. | |
| 1044 | - | leases = make(map[*attachedClient]*attachmentLease, len(c.leases)) | |
| 1044 | + | leases := make(map[*attachedClient]*attachmentLease, len(c.leases)) | |
| 1045 | 1045 | for ac, lease := range c.leases { | |
| 1046 | 1046 | if lease != nil && lease.active { | |
| 1047 | 1047 | leases[ac] = lease | |
| @@ -1005,21 +1005,21 @@ func (c *renderCoordinator) fireWithTimerTokenAndLease(token *timerToken, gen ui | |||||
| 1005 | if c.pendingGeneration == 0 { | 1005 | if c.pendingGeneration == 0 { | ||
| 1006 | c.pendingGeneration = 1 | 1006 | c.pendingGeneration = 1 | ||
| 1007 | } | 1007 | } | ||
| 1008 | - | leases := make(map[*attachedClient]*attachmentLease, len(c.leases)) | 1008 | + | // lease.ready is guarded by c.mu (markAttachmentReady writes it), so only |
| 1009 | + | // leases already ready under the lock are probed after unlocking. | |||
| 1010 | + | probe := make(map[*attachedClient]*attachmentLease, len(c.leases)) | |||
| 1009 | for ac, lease := range c.leases { | 1011 | for ac, lease := range c.leases { | ||
| 1010 | - | if lease != nil && lease.active { | 1012 | + | if lease != nil && lease.active && lease.ready { |
| 1011 | - | leases[ac] = lease | 1013 | + | probe[ac] = lease |
| 1012 | } | 1014 | } | ||
| 1013 | } | 1015 | } | ||
| 1014 | c.mu.Unlock() | 1016 | c.mu.Unlock() | ||
| 1015 | 1017 | ||||
| 1016 | - | ready := make(map[*attachmentLease]bool, len(leases)) | 1018 | + | ready := make(map[*attachmentLease]bool, len(probe)) |
| 1017 | - | for ac, lease := range leases { | 1019 | + | for ac, lease := range probe { |
| 1018 | // Welcome holds attachment.sendMu while its transport Send is in | 1020 | // Welcome holds attachment.sendMu while its transport Send is in | ||
| 1019 | // flight. Do not probe an incarnation before it is ready. | 1021 | // flight. Do not probe an incarnation before it is ready. | ||
| 1020 | - | if lease.ready { | 1022 | + | ready[lease] = c.attachmentAckReady(ac) |
| 1021 | - | ready[lease] = c.attachmentAckReady(ac) | |||
| 1022 | - | } | |||
| 1023 | } | 1023 | } | ||
| 1024 | 1024 | ||||
| 1025 | // The readiness callback can detach, replace, or publish a sync batch. | 1025 | // The readiness callback can detach, replace, or publish a sync batch. | ||
| @@ -1041,7 +1041,7 @@ func (c *renderCoordinator) fireWithTimerTokenAndLease(token *timerToken, gen ui | |||||
| 1041 | 1041 | ||||
| 1042 | // Re-snapshot after every external readiness probe. A replacement may have | 1042 | // Re-snapshot after every external readiness probe. A replacement may have | ||
| 1043 | // retired one of the leases we probed while the callback was unlocked. | 1043 | // retired one of the leases we probed while the callback was unlocked. | ||
| 1044 | - | leases = make(map[*attachedClient]*attachmentLease, len(c.leases)) | 1044 | + | leases := make(map[*attachedClient]*attachmentLease, len(c.leases)) |
| 1045 | for ac, lease := range c.leases { | 1045 | for ac, lease := range c.leases { | ||
| 1046 | if lease != nil && lease.active { | 1046 | if lease != nil && lease.active { | ||
| 1047 | leases[ac] = lease | 1047 | leases[ac] = lease | ||