Skip to content

Conversation

@dkovba
Copy link
Contributor

@dkovba dkovba commented Sep 17, 2025

The changes in this PR improve the consistency of using a lock in the ExecutionContext class.

@dkovba dkovba force-pushed the dkovba/execution-context branch from c41b54f to 87d49ec Compare September 17, 2025 01:30
/// - Throws: Any errors from the snapshotter
public func prepareSnapshot(for operationId: UUID) async throws -> Snapshot {
let parentCommitted = headSnapshot
let parentCommitted = lock.withLock { _headSnapshot }
Copy link
Contributor

@wlan0 wlan0 Sep 17, 2025

Choose a reason for hiding this comment

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

headSnapshot is already protected by a lock, is this necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The mixed use of headSnapshot and _headSnapshot might be confusing. But this is not incorrect.

Copy link
Contributor

@wlan0 wlan0 left a comment

Choose a reason for hiding this comment

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

one change seems redundant, but LGTM otherwise

@dkovba dkovba requested a review from wlan0 September 17, 2025 03:17
@dkovba dkovba merged commit c571546 into apple:main Sep 17, 2025
2 checks passed
@dkovba dkovba deleted the dkovba/execution-context branch September 17, 2025 16:49
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.

2 participants