From 29c92a10ea77accca068a7612931523de012c190 Mon Sep 17 00:00:00 2001 From: Slyghtning Date: Mon, 11 May 2026 16:56:42 +0200 Subject: [PATCH] instantout: detach reservation unlock cleanup context handleErrorAndUnlockReservations is called specifically from error paths and from the new OnRecover cleanup. In practice the caller's ctx is almost always already canceled by the time we get here (caller timeout, daemon shutdown, ctx.Done() arm in PollPaymentAcceptedAction, etc.). The existing implementation derived its 30s timeout context from that canceled parent, so: - The for-loop calling UnlockReservation immediately hit ctx.Err() == context.Canceled on every reservation. Locks were never released on disk. - The goroutine sending CancelInstantSwap to the server captured the same already-canceled ctx, then further wrapped it in WithTimeout (still canceled). The server never heard about the cancel. Both code paths were no-ops in exactly the scenario they were written for. Switch to context.Background() with a fresh 30s timeout so the cleanup actually runs. The goroutine also gets its own background context (the previous code captured the parent's already-done ctx via closure, then re-wrapped it). --- instantout/actions.go | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/instantout/actions.go b/instantout/actions.go index 6d3f403c..77820928 100644 --- a/instantout/actions.go +++ b/instantout/actions.go @@ -708,11 +708,17 @@ func (f *FSM) unlockReservationsOnRecoverAction(ctx context.Context, // handleErrorAndUnlockReservations handles an error and unlocks the // reservations. -func (f *FSM) handleErrorAndUnlockReservations(ctx context.Context, +func (f *FSM) handleErrorAndUnlockReservations(_ context.Context, err error) fsm.EventType { - // We might get here from a canceled context, we create a new context - // with a timeout to unlock the reservations. - ctx, cancel := context.WithTimeout(ctx, time.Second*30) + // We very likely got here from a canceled parent context (caller + // timeout, daemon shutdown, etc.). Deriving with timeout from a + // canceled parent yields an already-done context, so neither the + // local UnlockReservation calls nor the server-side CancelInstantSwap + // RPC would ever get a chance to run. Detach from the caller's + // context entirely. + ctx, cancel := context.WithTimeout( + context.Background(), time.Second*30, + ) defer cancel() // Unlock the reservations. @@ -727,19 +733,23 @@ func (f *FSM) handleErrorAndUnlockReservations(ctx context.Context, } // We're also sending the server a cancel message so that it can - // release the reservations. This can be done in a goroutine as we - // wan't to fail the fsm early. + // release the reservations. This runs in a goroutine because we + // want to fail the FSM early -- but it must use its OWN background + // context with timeout, not derive from the cancel above (which + // fires the moment this function returns). go func() { - ctx, cancel := context.WithTimeout(ctx, time.Second*30) + cancelCtx, cancel := context.WithTimeout( + context.Background(), time.Second*30, + ) defer cancel() _, cancelErr := f.cfg.InstantOutClient.CancelInstantSwap( - ctx, &swapserverrpc.CancelInstantSwapRequest{ + cancelCtx, &swapserverrpc.CancelInstantSwapRequest{ SwapHash: f.InstantOut.SwapHash[:], }, ) if cancelErr != nil { - // We'll log the error but not return it as we want to return the - // original error. + // We'll log the error but not return it as we want + // to return the original error. f.Debugf("error sending cancel message: %v", cancelErr) } }()