coracle/flotilla
1
6
Fork 23
Code Issues 42 Pull Requests 2 Actions Packages Projects 5 Releases Wiki Activity

Fix fallback pull race after abort #167

Merged
hodlbod merged 6 commits from nayan9617/flotilla:fix/pull-fallback-abort-race into dev 2026-04-08 16:43:05 +00:00
Conversation 8 Commits 6 Files Changed 2
+92 -47
nayan9617 commented 2026-04-07 17:50:12 +00:00
Contributor
Copy Link
Copy Source

Description

This PR fixes the sleep/wake sync leak by stopping stale async work in the fallback pull path from continuing after abort.

The investigation showed that welshman cleanup was already completing correctly, so the bug was not in request teardown. The real problem was in the async flow inside sync.ts: pullAndListen() was starting pullWithFallback() without awaiting it, and pullOneWithFallback() could continue past controller.abort() after loadRelay() resolved. That allowed the chain to reach Difference creation and fallback request() calls even though teardown had already won.

Fixes #111

What changed:

-Added an early signal.aborted guard at the top of pullOneWithFallback().
-Kept the abort check before Difference creation so no new listener work starts after teardown.
-Kept the abort check before the fallback request() call so the relay request is not re-issued after abort.
-Added an early guard in pullAndListen() so a stale call does not start the async chain at all.
-Kept the visibility lifecycle support already in place via state.ts.

Validation:

pnpm run check passed with 0 errors and 0 warnings.

## Description This PR fixes the sleep/wake sync leak by stopping stale async work in the fallback pull path from continuing after abort. The investigation showed that welshman cleanup was already completing correctly, so the bug was not in request teardown. The real problem was in the async flow inside sync.ts: pullAndListen() was starting pullWithFallback() without awaiting it, and pullOneWithFallback() could continue past controller.abort() after loadRelay() resolved. That allowed the chain to reach Difference creation and fallback request() calls even though teardown had already won. Fixes #111 ## What changed: -Added an early signal.aborted guard at the top of pullOneWithFallback(). -Kept the abort check before Difference creation so no new listener work starts after teardown. -Kept the abort check before the fallback request() call so the relay request is not re-issued after abort. -Added an early guard in pullAndListen() so a stale call does not start the async chain at all. -Kept the visibility lifecycle support already in place via state.ts. ## Validation: pnpm run check passed with 0 errors and 0 warnings.
hodlbod reviewed 2026-04-07 19:43:46 +00:00
hodlbod left a comment
Owner
Copy Link
Copy Source

Can you add .next to gitignore and remove it from the commit?

Otherwise this looks decent, but it occurs to me we probably actually want to keep syncing in the background, because that's how notifications get triggered. So I think the document visibility stuff is actually not something we should add. Sorry I didn't realize that earlier.

Can you add .next to gitignore and remove it from the commit? Otherwise this looks decent, but it occurs to me we probably actually want to keep syncing in the background, because that's how notifications get triggered. So I think the document visibility stuff is actually not something we should add. Sorry I didn't realize that earlier.
src/app/core/state.ts Outdated
@@ -159,0 +160,4 @@
export const documentVisibility = readable<DocumentVisibilityState>(
typeof document !== "undefined" ? document.visibilityState : "visible",
set => {
if (typeof document === "undefined") return
hodlbod commented 2026-04-07 19:36:15 +00:00
Owner
Copy Link
Copy Source

No need to check for document, this project doesn't do SSR

No need to check for document, this project doesn't do SSR
src/app/core/sync.ts Outdated
@@ -6,6 +6,7 @@ import {PollResponse} from "nostr-tools/kinds"
import {
getListTags,
getRelayTagValues,
type List,
hodlbod commented 2026-04-07 19:35:57 +00:00
Owner
Copy Link
Copy Source

Use import type rather than mixing type/variable imports

Use `import type` rather than mixing type/variable imports
hodlbod marked this conversation as resolved
src/app/core/sync.ts Outdated
@@ -240,3 +241,1 @@
loadFeedsForPubkey($userRelayList.event.pubkey)
}
})
const syncRelayList = ($userRelayList: List | undefined) => {
hodlbod commented 2026-04-07 19:37:45 +00:00
Owner
Copy Link
Copy Source

You can use PublishedList here for a slightly more narrow type

You can use `PublishedList` here for a slightly more narrow type
hodlbod marked this conversation as resolved
src/app/core/sync.ts Outdated
@@ -245,3 +257,2 @@
for (const pubkeys of chunk(10, get(bootstrapPubkeys))) {
// This isn't urgent, avoid clogging other stuff up
await sleep(1000)
void (async () => {
hodlbod commented 2026-04-07 19:39:23 +00:00
Owner
Copy Link
Copy Source

This actually changes the behavior to flood relays with requests. This needs to be a trickle in the background to prevent interrupting other requests. If we need to cancel it, then we should add that explicitly.

This actually changes the behavior to flood relays with requests. This needs to be a trickle in the background to prevent interrupting other requests. If we need to cancel it, then we should add that explicitly.
hodlbod marked this conversation as resolved
src/app/core/sync.ts Outdated
@@ -260,0 +293,4 @@
},
)
const unsubscribeFollows = derived([userFollowList, documentVisibility], identity).subscribe(
hodlbod commented 2026-04-07 19:40:13 +00:00
Owner
Copy Link
Copy Source

Instead of derived/identity you can use merged from @welshman/store

Instead of derived/identity you can use `merged` from `@welshman/store`
hodlbod marked this conversation as resolved
hodlbod commented 2026-04-08 16:23:33 +00:00
Owner
Copy Link
Copy Source

.next is still in the diff

.next is still in the diff
nayan9617 commented 2026-04-08 16:29:00 +00:00
Author
Contributor
Copy Link
Copy Source

Actually that was due to earlier commit, and despite adding it to .gitignore it was still there. Though, I've now untracked those files.

Actually that was due to earlier commit, and despite adding it to .gitignore it was still there. Though, I've now untracked those files.
hodlbod added 6 commits 2026-04-08 16:42:56 +00:00
fix: reset relay sockets on visibility change to prevent auth hangs on sleep/wake a5506689f7 
When browser tab is hidden (sleep), tear down all relay socket connections
completely. This forces fresh socket creation and clean auth handshake on wake,
preventing stuck AuthStatus.PendingResponse states.

- Add documentVisibility store tracking document.visibilityState
- Call Pool.get().remove(url) for relay sockets during hidden teardown
- Integrate socket reset in syncUserData, syncSpaces, syncDMs
- Add explanatory comments for visibility guards and race conditions

Fixes: App stuck in "Authenticating" after Mac sleep/wake cycle
Tested with: wss://news.utxo.one/
refactor: revert socket reset workaround for adapter layer investigation e710fc2561
Guard fallback pulls after abort 35c27090c1
Align sync as per the review 2144bffc41
Keep follow sync trickling 55497b7949
Stop tracking .next artifacts 3dd5a0915f
hodlbod force-pushed fix/pull-fallback-abort-race from d0e23b605c to 3dd5a0915f 2026-04-08 16:42:56 +00:00 Compare
hodlbod merged commit 7f1e98dcb2 into dev 2026-04-08 16:43:05 +00:00
hodlbod deleted branch fix/pull-fallback-abort-race 2026-04-08 16:43:05 +00:00
hodlbod commented 2026-04-08 16:43:10 +00:00
Owner
Copy Link
Copy Source

Perfect, thanks!

Perfect, thanks!
userAdityaa commented 2026-04-08 17:38:35 +00:00
Collaborator
Copy Link
Copy Source

Hey @hodlbod, I wasn’t experiencing any logging-related hangs before, but after pulling this merge I’m now running into an issue where it gets stuck.

Could you please take a look? I’ve attached a video for reference.

Hey @hodlbod, I wasn’t experiencing any logging-related hangs before, but after pulling this merge I’m now running into an issue where it gets stuck. Could you please take a look? I’ve attached a video for reference. <video src="attachments/6235341c-50bd-4a13-90ce-b0697831d167" title="Screen Recording 2026-04-08 at 11.17.58 PM.mov" controls></video>
Screen Recording 2026-04-08 at 11.17.58 PM.mov
3.5 MiB
userAdityaa commented 2026-04-08 17:39:38 +00:00
Collaborator
Copy Link
Copy Source

update: It worked, but it took quite a while.

**update:** It worked, but it took quite a while.
nayan9617 commented 2026-04-08 17:42:40 +00:00
Author
Contributor
Copy Link
Copy Source

hello @userAdityaa thanks for letting us know. Let me check if this could anyway affect logging delays.

hello @userAdityaa thanks for letting us know. Let me check if this could anyway affect logging delays.
hodlbod commented 2026-04-08 17:48:39 +00:00
Owner
Copy Link
Copy Source

The login issue is unrelated to this PR, it looks like one of the signers is timing out. I'll look into it.

The login issue is unrelated to this PR, it looks like one of the signers is timing out. I'll look into it.
👍 1
Sign in to join this conversation.
Reviewers
No Reviewers
hodlbod
Labels
Clear labels
design
For issues related to design work. Development should be done in another issue and refer back to this one.
 dev
Issues related to development work
 easy
Issues that can be executed on by new contributors
 idea
For issues that aren't yet actionable
 priority
For issues related to high priority features and bugs
 usability
Issues that have to be validated with real live human beings.
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
hodlbod (hodlbod) mplorentz (Matt Lorentz) sakshamjain (Saksham Jain) sondreb triesap userAdityaa (Aditya Chaudhary)
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: coracle/flotilla#167
Delete Branch "nayan9617/flotilla:fix/pull-fallback-abort-race"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?