fix(daemon): refuse a same-peer switch during a purge as unavailable and keep its offer
- commit
- 1d86a915a91a3574d16123d5fe481e4a73786532
- author
- brice · · 2026-10-08 08:55 UTC
- committer
- brice · · 2026-10-08 09:31 UTC
- parent
- c002b88
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +18 -10
M internal/usecase/daemon/same_peer_switch.go +8 -7
| @@ -69,17 +69,13 @@ func (d *Daemon) switchSamePeerForAttachment(effect *attachmentEffect, request p | |||
| 69 | 69 | if err := request.Validate(); err != nil || !effect.current() || effect.sess == nil || effect.ac == nil { | |
| 70 | 70 | return | |
| 71 | 71 | } | |
| 72 | - | if !effect.ac.consumeSamePeerOffer(request.Target) { | |
| 73 | - | d.sendSamePeerSwitchFailure(effect, request.RequestID, protocol.SamePeerSwitchStaleTarget) | |
| 74 | - | return | |
| 75 | - | } | |
| 76 | - | ||
| 77 | 72 | // The switch moves an attachment between sessions, so it must not run | |
| 78 | - | // inside the exact set a KillAll purge has captured. | |
| 73 | + | // inside the exact set a KillAll purge has captured. A purge is retry | |
| 74 | + | // state, so it is checked first and keeps any pending daemon offer. | |
| 79 | 75 | d.mu.Lock() | |
| 80 | 76 | if d.purgeAdmissionClosedLocked() { | |
| 81 | 77 | d.mu.Unlock() | |
| 82 | - | d.sendSamePeerSwitchFailure(effect, request.RequestID, protocol.SamePeerSwitchStaleTarget) | |
| 78 | + | d.sendSamePeerSwitchFailure(effect, request.RequestID, protocol.SamePeerSwitchUnavailable) | |
| 83 | 79 | return | |
| 84 | 80 | } | |
| 85 | 81 | d.purgeAdmissionActive++ | |
| @@ -90,6 +86,11 @@ func (d *Daemon) switchSamePeerForAttachment(effect *attachmentEffect, request p | |||
| 90 | 86 | release := func() { releaseOnce.Do(d.releasePurgeAdmission) } | |
| 91 | 87 | defer release() | |
| 92 | 88 | ||
| 89 | + | if !effect.ac.consumeSamePeerOffer(request.Target) { | |
| 90 | + | d.sendSamePeerSwitchFailure(effect, request.RequestID, protocol.SamePeerSwitchStaleTarget) | |
| 91 | + | return | |
| 92 | + | } | |
| 93 | + | ||
| 93 | 94 | target, targetTabIndex, ok := d.samePeerTarget(request) | |
| 94 | 95 | if !ok || target == effect.sess { | |
| 95 | 96 | d.sendSamePeerSwitchFailure(effect, request.RequestID, protocol.SamePeerSwitchStaleTarget) | |
| @@ -69,17 +69,13 @@ func (d *Daemon) switchSamePeerForAttachment(effect *attachmentEffect, request p | |||||
| 69 | if err := request.Validate(); err != nil || !effect.current() || effect.sess == nil || effect.ac == nil { | 69 | if err := request.Validate(); err != nil || !effect.current() || effect.sess == nil || effect.ac == nil { | ||
| 70 | return | 70 | return | ||
| 71 | } | 71 | } | ||
| 72 | - | if !effect.ac.consumeSamePeerOffer(request.Target) { | |||
| 73 | - | d.sendSamePeerSwitchFailure(effect, request.RequestID, protocol.SamePeerSwitchStaleTarget) | |||
| 74 | - | return | |||
| 75 | - | } | |||
| 76 | - | ||||
| 77 | // The switch moves an attachment between sessions, so it must not run | 72 | // The switch moves an attachment between sessions, so it must not run | ||
| 78 | - | // inside the exact set a KillAll purge has captured. | 73 | + | // inside the exact set a KillAll purge has captured. A purge is retry |
| 74 | + | // state, so it is checked first and keeps any pending daemon offer. | |||
| 79 | d.mu.Lock() | 75 | d.mu.Lock() | ||
| 80 | if d.purgeAdmissionClosedLocked() { | 76 | if d.purgeAdmissionClosedLocked() { | ||
| 81 | d.mu.Unlock() | 77 | d.mu.Unlock() | ||
| 82 | - | d.sendSamePeerSwitchFailure(effect, request.RequestID, protocol.SamePeerSwitchStaleTarget) | 78 | + | d.sendSamePeerSwitchFailure(effect, request.RequestID, protocol.SamePeerSwitchUnavailable) |
| 83 | return | 79 | return | ||
| 84 | } | 80 | } | ||
| 85 | d.purgeAdmissionActive++ | 81 | d.purgeAdmissionActive++ | ||
| @@ -90,6 +86,11 @@ func (d *Daemon) switchSamePeerForAttachment(effect *attachmentEffect, request p | |||||
| 90 | release := func() { releaseOnce.Do(d.releasePurgeAdmission) } | 86 | release := func() { releaseOnce.Do(d.releasePurgeAdmission) } | ||
| 91 | defer release() | 87 | defer release() | ||
| 92 | 88 | ||||
| 89 | + | if !effect.ac.consumeSamePeerOffer(request.Target) { | |||
| 90 | + | d.sendSamePeerSwitchFailure(effect, request.RequestID, protocol.SamePeerSwitchStaleTarget) | |||
| 91 | + | return | |||
| 92 | + | } | |||
| 93 | + | ||||
| 93 | target, targetTabIndex, ok := d.samePeerTarget(request) | 94 | target, targetTabIndex, ok := d.samePeerTarget(request) | ||
| 94 | if !ok || target == effect.sess { | 95 | if !ok || target == effect.sess { | ||
| 95 | d.sendSamePeerSwitchFailure(effect, request.RequestID, protocol.SamePeerSwitchStaleTarget) | 96 | d.sendSamePeerSwitchFailure(effect, request.RequestID, protocol.SamePeerSwitchStaleTarget) | ||
M internal/usecase/daemon/same_peer_switch_test.go +10 -3
| @@ -133,11 +133,13 @@ func TestSamePeerSwitchClientInitiated(t *testing.T) { | |||
| 133 | 133 | offer *protocol.ExactSessionTarget | |
| 134 | 134 | purging bool | |
| 135 | 135 | switched bool | |
| 136 | + | wantCode protocol.SamePeerSwitchFailureCode | |
| 136 | 137 | }{ | |
| 137 | 138 | {name: "no pending offer switches", switched: true}, | |
| 138 | 139 | {name: "matching offer switches", offer: &requestTarget, switched: true}, | |
| 139 | - | {name: "another pending offer refuses", offer: &otherOffer}, | |
| 140 | - | {name: "a running purge refuses", purging: true}, | |
| 140 | + | {name: "another pending offer refuses", offer: &otherOffer, wantCode: protocol.SamePeerSwitchStaleTarget}, | |
| 141 | + | {name: "a running purge refuses", purging: true, wantCode: protocol.SamePeerSwitchUnavailable}, | |
| 142 | + | {name: "a running purge keeps the pending offer", offer: &requestTarget, purging: true, wantCode: protocol.SamePeerSwitchUnavailable}, | |
| 141 | 143 | } | |
| 142 | 144 | for _, tt := range tests { | |
| 143 | 145 | t.Run(tt.name, func(t *testing.T) { | |
| @@ -179,7 +181,12 @@ func TestSamePeerSwitchClientInitiated(t *testing.T) { | |||
| 179 | 181 | } | |
| 180 | 182 | require.Same(t, source, ac.currentAttachmentSession()) | |
| 181 | 183 | failure := decodeServerMessage(t, awaitFrame(t, sends, "SamePeerSwitchFailure")).(protocol.SamePeerSwitchFailure) | |
| 182 | - | require.Equal(t, protocol.SamePeerSwitchStaleTarget, failure.Code) | |
| 184 | + | require.Equal(t, tt.wantCode, failure.Code) | |
| 185 | + | if tt.offer != nil { | |
| 186 | + | ac.samePeerOfferMu.Lock() | |
| 187 | + | defer ac.samePeerOfferMu.Unlock() | |
| 188 | + | require.Equal(t, tt.offer, ac.samePeerOffer, "a refused switch keeps the pending offer") | |
| 189 | + | } | |
| 183 | 190 | }) | |
| 184 | 191 | } | |
| 185 | 192 | } | |
| @@ -133,11 +133,13 @@ func TestSamePeerSwitchClientInitiated(t *testing.T) { | |||||
| 133 | offer *protocol.ExactSessionTarget | 133 | offer *protocol.ExactSessionTarget | ||
| 134 | purging bool | 134 | purging bool | ||
| 135 | switched bool | 135 | switched bool | ||
| 136 | + | wantCode protocol.SamePeerSwitchFailureCode | |||
| 136 | }{ | 137 | }{ | ||
| 137 | {name: "no pending offer switches", switched: true}, | 138 | {name: "no pending offer switches", switched: true}, | ||
| 138 | {name: "matching offer switches", offer: &requestTarget, switched: true}, | 139 | {name: "matching offer switches", offer: &requestTarget, switched: true}, | ||
| 139 | - | {name: "another pending offer refuses", offer: &otherOffer}, | 140 | + | {name: "another pending offer refuses", offer: &otherOffer, wantCode: protocol.SamePeerSwitchStaleTarget}, |
| 140 | - | {name: "a running purge refuses", purging: true}, | 141 | + | {name: "a running purge refuses", purging: true, wantCode: protocol.SamePeerSwitchUnavailable}, |
| 142 | + | {name: "a running purge keeps the pending offer", offer: &requestTarget, purging: true, wantCode: protocol.SamePeerSwitchUnavailable}, | |||
| 141 | } | 143 | } | ||
| 142 | for _, tt := range tests { | 144 | for _, tt := range tests { | ||
| 143 | t.Run(tt.name, func(t *testing.T) { | 145 | t.Run(tt.name, func(t *testing.T) { | ||
| @@ -179,7 +181,12 @@ func TestSamePeerSwitchClientInitiated(t *testing.T) { | |||||
| 179 | } | 181 | } | ||
| 180 | require.Same(t, source, ac.currentAttachmentSession()) | 182 | require.Same(t, source, ac.currentAttachmentSession()) | ||
| 181 | failure := decodeServerMessage(t, awaitFrame(t, sends, "SamePeerSwitchFailure")).(protocol.SamePeerSwitchFailure) | 183 | failure := decodeServerMessage(t, awaitFrame(t, sends, "SamePeerSwitchFailure")).(protocol.SamePeerSwitchFailure) | ||
| 182 | - | require.Equal(t, protocol.SamePeerSwitchStaleTarget, failure.Code) | 184 | + | require.Equal(t, tt.wantCode, failure.Code) |
| 185 | + | if tt.offer != nil { | |||
| 186 | + | ac.samePeerOfferMu.Lock() | |||
| 187 | + | defer ac.samePeerOfferMu.Unlock() | |||
| 188 | + | require.Equal(t, tt.offer, ac.samePeerOffer, "a refused switch keeps the pending offer") | |||
| 189 | + | } | |||
| 183 | }) | 190 | }) | ||
| 184 | } | 191 | } | ||
| 185 | } | 192 | } | ||