Skip to content

Commit

Permalink
Address read-lock-write race condition in WorkspaceSnapshot::revert
Browse files Browse the repository at this point in the history
We had a similar race condition in WorkspaceSnapshot::working_copy_mut,
where we were checking to see if we needed to do something, then
acquiring the write lock necessary to do the work if the initial check
determined we needed to do it. Unfortunately, this means that the state
of the thing being checked can change between when the conditions are
checked and the write lock is acquired, as it is possible for another
writer (or even multiple) to acquire the write lock and change things
before the initial (potential) writer is able to act on the check(s) it
did.

The way around this is to assume that a write will be necessary, acquire
the write lock, and do the checks within the write lock. If the
modification turns out to be unnecessary, then no modification is done,
but if modification is necessary, then it is not possible for another
writer to affect things before we are able to do our change(s) as we
already have the write lock.
  • Loading branch information
jhelwig committed Aug 22, 2024
1 parent bd62702 commit 314fd9e
Showing 1 changed file with 3 additions and 2 deletions.
5 changes: 3 additions & 2 deletions lib/dal/src/workspace_snapshot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -493,8 +493,9 @@ impl WorkspaceSnapshot {
/// Discard all changes in the working copy and return the graph to the
/// version fetched from the layer db
pub async fn revert(&self) {
if self.working_copy.read().await.is_some() {
*self.working_copy.write().await = None
let mut working_copy = self.working_copy.write().await;
if working_copy.is_some() {
*working_copy = None;
}
}

Expand Down

0 comments on commit 314fd9e

Please sign in to comment.