Skip to content

fix: make FileSystem init directory creation race-safe - #35

Merged
VenkataKrishnaGarapati merged 2 commits into
DuetHealth:mainfrom
VenkataKrishnaGarapati:hotfix/filesystem-init-race
Jun 4, 2026
Merged

fix: make FileSystem init directory creation race-safe#35
VenkataKrishnaGarapati merged 2 commits into
DuetHealth:mainfrom
VenkataKrishnaGarapati:hotfix/filesystem-init-race

Conversation

@VenkataKrishnaGarapati

@VenkataKrishnaGarapati VenkataKrishnaGarapati commented Jun 3, 2026

Copy link
Copy Markdown
Contributor

Technical Changes

  • Replaced the check-then-create pattern in FileSystem.init with idempotent directory creation.
  • Removed fatal try! trap risk during concurrent cache initialization by using try? createDirectory(..., withIntermediateDirectories: true, ...).
  • Added regression tests in Tests/KumoTests/Fixtures/Blobs/FileSystemInitializationTests.swift for:
    • file-exists error path during initialization
    • idempotent initialization when directory already exists

Reason

This addresses launch-time crashes caused by a race in cache directory creation (BlobCache/FileSystem init) under concurrent first access.

Validation

  • Ran: swift test --filter FileSystemInitializationTests
  • Result: 2 tests passed, 0 failures

@powerje

powerje commented Jun 3, 2026

Copy link
Copy Markdown
Collaborator

lgtm!

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the blob cache’s filesystem-backed storage initialization to avoid a check-then-create race during concurrent startup access, and adds targeted regression tests to cover the initialization scenarios.

Changes:

  • Updated FileSystem initialization to use idempotent directory creation (avoiding the prior check-then-create race).
  • Added XCTest coverage for initialization when directory creation reports “file exists” and when the directory already exists.

Reviewed changes

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

File Description
Sources/Kumo/Blobs/Storage/FileSystem.swift Makes cache directory setup idempotent to prevent concurrent initialization crashes.
Tests/KumoTests/Fixtures/Blobs/FileSystemInitializationTests.swift Adds regression tests for initialization behavior around directory creation/existence.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread Sources/Kumo/Blobs/Storage/FileSystem.swift Outdated
Comment thread Tests/KumoTests/Fixtures/Blobs/FileSystemInitializationTests.swift

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 2 out of 2 changed files in this pull request and generated 2 comments.

Comment thread Sources/Kumo/Blobs/Storage/FileSystem.swift
Comment thread Tests/KumoTests/Fixtures/Blobs/FileSystemInitializationTests.swift

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 2 out of 2 changed files in this pull request and generated 1 comment.

Comment thread Sources/Kumo/Blobs/Storage/FileSystem.swift

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 2 out of 2 changed files in this pull request and generated 2 comments.

Comment thread Sources/Kumo/Blobs/Storage/FileSystem.swift Outdated
Comment on lines +34 to +35
throw NSError(domain: NSCocoaErrorDomain, code: 516)
}
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
@VenkataKrishnaGarapati
VenkataKrishnaGarapati merged commit 1767665 into DuetHealth:main Jun 4, 2026
15 of 20 checks passed
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.

3 participants