Skip to content

Commit 3f6b1b0

Browse files
committed
SMB reconnect: Guard handleDirect against double-fire
Both `runAttempt`'s success branch and the `smb-connection-changed` event listener can call `handleDirect` for the same cycle, which would fire `onSuccess` twice and double-trigger `loadDirectory` in the FilePane. The new guard checks the entry's baseline shape (`status: 'waiting'`, no timer, `attemptIndex: 0`) and returns early when there's nothing to clean up. The runAttempt-side call stays as a defensive backstop — `handleDirect` is now safely idempotent. Adds a regression test that triggers both paths in the same cycle and asserts `onSuccess` fires exactly once.
1 parent 0c1d368 commit 3f6b1b0

2 files changed

Lines changed: 27 additions & 5 deletions

File tree

apps/desktop/src/lib/file-explorer/network/smb-reconnect-manager.svelte.test.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -175,6 +175,24 @@ describe('smbReconnectManager', () => {
175175
expect(smbReconnectManager.getState('vol-refcount')).toBeNull()
176176
})
177177

178+
it('handleDirect is idempotent — onSuccess fires exactly once per cycle', async () => {
179+
// Race scenario: both the `direct` event and the awaited `reconnectSmbVolume`
180+
// success path could each trigger `handleDirect`. The idempotency guard
181+
// ensures `onSuccess` only fires once.
182+
await smbReconnectManager.init()
183+
const onSuccess = vi.fn()
184+
const unsub = smbReconnectManager.subscribe('vol-once', onSuccess)
185+
smbReconnectManager.startCycle('vol-once')
186+
// Resolve `reconnectSmbVolume` AND emit the `direct` event (simulating both
187+
// paths racing to clean up). Only one should win and notify.
188+
mockReconnect.mockResolvedValueOnce(undefined)
189+
await vi.advanceTimersByTimeAsync(RECONNECT_DELAYS_MS[0])
190+
emit('vol-once', 'direct')
191+
expect(onSuccess).toHaveBeenCalledTimes(1)
192+
193+
unsub()
194+
})
195+
178196
it('two subscribers see the same state object (one cycle, both panes)', async () => {
179197
await smbReconnectManager.init()
180198
const unsub1 = smbReconnectManager.subscribe('vol-shared')

apps/desktop/src/lib/file-explorer/network/smb-reconnect-manager.svelte.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,12 @@ class SmbReconnectManager {
191191
private handleDirect(volumeId: string): void {
192192
const entry = this.map.get(volumeId)
193193
if (!entry) return
194+
// Idempotent: if no cycle is in flight (state is the baseline + no timer),
195+
// there's nothing to clean up and no subscribers to notify. This guards
196+
// against double-firing when both `runAttempt`'s success branch and the
197+
// `smb-connection-changed` event fire — whichever runs first wins.
198+
const noActiveCycle = entry.state.status === 'waiting' && entry.timerId === null && entry.state.attemptIndex === 0
199+
if (noActiveCycle) return
194200
if (entry.timerId) clearTimeout(entry.timerId)
195201
entry.timerId = null
196202
entry.state = freshState()
@@ -238,11 +244,9 @@ class SmbReconnectManager {
238244

239245
try {
240246
await reconnectSmbVolume(volumeId)
241-
// Success — let the `smb-connection-changed` listener handle cleanup so
242-
// the path that runs is the same whether success comes from us or from
243-
// an unrelated trigger (lazy nav, manual reconnect from another window).
244-
// As a defensive backstop in case the event never arrives, also clean up
245-
// here. `handleDirect` is idempotent.
247+
// Success — defensive backstop in case the `smb-connection-changed`
248+
// event somehow doesn't arrive (unexpected, but `handleDirect` is
249+
// idempotent so calling both paths is safe).
246250
this.handleDirect(volumeId)
247251
} catch (e) {
248252
log.info('Reconnect attempt {attempt} for {volumeId} failed: {error}', {

0 commit comments

Comments
 (0)