Skip to content

Fix race conditions interacting with Cloud SQLite jobs#9207

Merged
tcobbs-bentley merged 13 commits intomasterfrom
tcobbs/cloudsqlite-job-race-fix
Apr 17, 2026
Merged

Fix race conditions interacting with Cloud SQLite jobs#9207
tcobbs-bentley merged 13 commits intomasterfrom
tcobbs/cloudsqlite-job-race-fix

Conversation

@tcobbs-bentley
Copy link
Copy Markdown
Member

There was a race condition where the progress callback handler could happen after the Cloud SQLite job had completed, and this would lead to an unhandled exception thrown from a timer callback.

There was a race condition where the progress callback handler could happen after the Cloud SQLite job had completed, and this would lead to an unhandled exception thrown from a timer callback.
@tcobbs-bentley tcobbs-bentley requested a review from a team as a code owner April 16, 2026 19:08
@tcobbs-bentley
Copy link
Copy Markdown
Member Author

@khanaffan @wgoehrig This really needs a new unit test, but I don't know how to create a unit test that forces the race condition to occur. Do you have any suggestions? I could probably force it to happen in the delete case by having a progress callback that sleeps and then returns 1, but I have no idea how to force it to happen in any other case.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes a backend race where Cloud SQLite job progress polling could outlive job completion and throw from a timer callback, leading to an unhandled exception.

Changes:

  • Added a helper to recognize the native “transfer already completed” error.
  • Wrapped setInterval progress polling for cleanup and transfer jobs in try/catch to ignore that completion race.
  • Updated imports to use ITwinError for error-shape checking.

Comment thread core/backend/src/CloudSqlite.ts Outdated
Comment thread core/backend/src/CloudSqlite.ts
Comment thread core/backend/src/CloudSqlite.ts Outdated
Comment thread full-stack-tests/backend/src/integration/CloudSqlite.test.ts Fixed
Comment thread full-stack-tests/backend/src/integration/CloudSqlite.test.ts Fixed
tcobbs-bentley and others added 2 commits April 16, 2026 14:47
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <223894421+github-code-quality[bot]@users.noreply.github.com>
@tcobbs-bentley
Copy link
Copy Markdown
Member Author

@wgoehrig @khanaffan I added an integration test that uses a slow onProgress callback to force the race condition. I verified that my test fails due to the error without my fix, and passes with my fix. I can't think of any way to trigger the race condition under normal operation, meaning I don't know how to add a test for transferDb or for a typical cleanDeletedBlocks run.

* Add a new `UtilityFunctions.ts` to core-bentley with an `intervalWrapper` function
* Wrap the CloudSqlite `setInterval` callback code using `setInterval` so that any exceptions thrown will propagate to the main promise chain
* Update the integration test to fail if `cleanDeletedBlocks` throws an exception
@khanaffan
Copy link
Copy Markdown
Contributor

This is a common problem when using timer to simulate event or progress. It can fire after the resource is disposed. I think your fix is fine for what we have right now.

@tcobbs-bentley tcobbs-bentley enabled auto-merge (squash) April 17, 2026 18:11
Comment thread core/bentley/src/UtilityFunctions.ts Outdated
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 6 comments.

Comment thread full-stack-tests/backend/src/integration/CloudSqlite.test.ts
Comment thread full-stack-tests/backend/src/integration/CloudSqlite.test.ts
Comment thread core/backend/src/CloudSqlite.ts Outdated
Comment thread core/backend/src/CloudSqlite.ts Outdated
Comment thread core/backend/src/CloudSqlite.ts Outdated
Comment thread full-stack-tests/backend/src/integration/CloudSqlite.test.ts
@tcobbs-bentley tcobbs-bentley merged commit 243f6f9 into master Apr 17, 2026
18 checks passed
@tcobbs-bentley tcobbs-bentley deleted the tcobbs/cloudsqlite-job-race-fix branch April 17, 2026 20:30
@GytisCepk
Copy link
Copy Markdown
Contributor

@Mergifyio backport release/5.8.x

@mergify
Copy link
Copy Markdown
Contributor

mergify Bot commented Apr 23, 2026

backport release/5.8.x

✅ Backports have been created

Details

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

CloudSQLite cleanDeletedBlocks throws "transfer already completed"

5 participants