Skip to content

fix: guard debounced save against concurrent PUT calls#140

Merged
kptdobe merged 1 commit into
mainfrom
fix/no-concurrent-save
May 11, 2026
Merged

fix: guard debounced save against concurrent PUT calls#140
kptdobe merged 1 commit into
mainfrom
fix/no-concurrent-save

Conversation

@kptdobe
Copy link
Copy Markdown
Contributor

@kptdobe kptdobe commented May 11, 2026

Problem

The debounced update handler in persistence.bindState is an async function called via lodash.debounce. If a PUT takes longer than the debounce window (2 s max-wait is 10 s), a second firing of the debounced callback will race the first — both simultaneously awaiting persistence.put against da-admin. This can cause:

  • Out-of-order writes (last PUT wins, earlier content lost)
  • Duplicate PUT traffic under high update rates

Fix

Introduce a saving boolean flag in the closure. If the flag is set when the debounced callback fires, the second invocation returns immediately instead of launching a concurrent PUT.

let saving = false;
ydoc.on('update', debounce(async () => {
  if (saving || !current || ydoc !== docs.get(docName)) return;
  saving = true;
  try {
    current = await persistence.update(ydoc, current, docName);
  } finally {
    saving = false;
  }
}, 2000, { maxWait: 10000 }));

Test

New test Test concurrent save calls are guarded by saving flag:

  • Mocks debounce as identity so the handler fires immediately
  • Mocks persistence.put with a 30 ms artificial delay and a concurrency counter
  • Fires the debounced handler twice concurrently (Promise.all)
  • Asserts maxConcurrentPuts === 1

The test fails on the old code (observed 12 concurrent puts) and passes after this fix.

Part of series

This is PR 4/4 fixing the image-disappears-on-drag-and-drop bug:

🤖 Generated with Claude Code

@bosschaert
Copy link
Copy Markdown
Contributor

Seems that this PR contains more than just what is described? Seems like it also contains the changes from #137 ?

It think it would be better to isolate the changes 😄

@kptdobe kptdobe changed the base branch from main to fix/no-ifmatch-412 May 11, 2026 07:47
Add a `saving` flag to the debounced da-admin update handler in
`bindState` so that if a prior PUT is still in flight when the
debounced callback fires again, the second invocation is skipped
rather than racing the first.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@kptdobe kptdobe force-pushed the fix/no-ifmatch-412 branch from e6e9fe6 to c728a3b Compare May 11, 2026 07:55
@kptdobe kptdobe force-pushed the fix/no-concurrent-save branch from 35a0794 to b2a51d5 Compare May 11, 2026 07:55
@kptdobe kptdobe changed the base branch from fix/no-ifmatch-412 to main May 11, 2026 07:55
@kptdobe
Copy link
Copy Markdown
Contributor Author

kptdobe commented May 11, 2026

@bosschaert yes, for some reason, the PR became cumulative. I isolate the change now, it only contains the PR specific changes only.

@kptdobe kptdobe merged commit 3eba8af into main May 11, 2026
9 of 11 checks passed
@kptdobe kptdobe deleted the fix/no-concurrent-save branch May 11, 2026 17:42
adobe-bot pushed a commit that referenced this pull request May 11, 2026
## [1.2.5](v1.2.4...v1.2.5) (2026-05-11)

### Bug Fixes

* guard debounced save against concurrent PUT calls ([#140](#140)) ([3eba8af](3eba8af))
@adobe-bot
Copy link
Copy Markdown
Collaborator

🎉 This PR is included in version 1.2.5 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants