zed-industries/zed
1
1
Fork 0
mirror of https://github.com/zed-industries/zed.git synced 2024-11-10 05:37:29 +03:00
Code Issues Packages Projects Releases Wiki Activity

Fix file-reloading race condition (#3348)

Browse Source 
### Summary

This PR fixes a bug that @as-cii and @osiewicz saw when the on-disk
contents of files changed due to running `git checkout` at the command
line. It caused a buffer's contents to diverge from the file's on disk
contents, but the buffer to show an *unmodified* status.

I've also introduced new APIs on gpui's deterministic executor, which
make it possible to write a test that reliably triggered the bug.

### Details

The bug is triggered by the following sequence of events:
1. A buffer's file changes on disk while the buffer is *unmodified*
2. Zed reloads the new content of the file
3. Before updating the buffer itself, Zed computes a *diff* between the
buffer's current contents, and the newly-loaded contents
4. While this diff is being computed, one of two things happens:
     1. the buffer changes on-disk *again*.
2. the user edits the buffer, but undoes the edit, so that the buffer
returns to an unmodified state

The bug itself was caused by a few things:
* The buffer diffing algorithm is pretty slow, because we perform a
character-wise diff
* We previously allowed multiple reload tasks to run concurrently
* When discarding an out-of-date diff, we failed to update parts of the
buffer's state (`saved_fingerprint`) which allow us to recognize that
the buffer's content differs from the file.

It was also difficult to reproduce the problem in tests, because under
deterministic execution, because it was extremely unlikely for other
tasks to make progress *after* a file had been reloaded, but *before*
the disk task has resolved. To help with testing, I introduced a pair of
executor APIs:

`spawn_labeled`, - for spawning a background task with a given *label*
`deprioritize_task` - for forcing tasks with a given label to run
*after* all other concurrent tasks.

I also made the `Model::next_event` test helper method more useful, in
that it no longer runs *until* parked in order to wait for the next
event to occur. It just steps the executor one poll at a time until the
model emits an event.

Release Notes:

- Fixed a bug that caused buffers to report incorrect modified/conflict
status when their buffers changed on disk multiple times in rapid
succession.
This commit is contained in:
Max Brunsfeld 2023-11-16 20:19:16 -08:00 committed by GitHub
commit e67c44a562
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 404 additions and 191 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
crates/copilot/src/copilot.rs
View File

@ -1051,17 +1051,15 @@ mod tests {
);
// Ensure updates to the file are reflected in the LSP.
buffer_1
.update(cx, |buffer, cx| {
buffer.file_updated(
Arc::new(File {
abs_path: "/root/child/buffer-1".into(),
path: Path::new("child/buffer-1").into(),
}),
cx,
)
})
.await;
buffer_1.update(cx, |buffer, cx| {
buffer.file_updated(
Arc::new(File {
abs_path: "/root/child/buffer-1".into(),
path: Path::new("child/buffer-1").into(),
}),
cx,
)
});
assert_eq!(
lsp.receive_notification::<lsp::notification::DidCloseTextDocument>()
.await,

17
crates/gpui2/src/app/test_context.rs
View File

@ -370,10 +370,19 @@ impl<T: Send> Model<T> {
})
});
cx.executor().run_until_parked();
rx.try_next()
.expect("no event received")
.expect("model was dropped")
// Run other tasks until the event is emitted.
loop {
match rx.try_next() {
Ok(Some(event)) => return event,
Ok(None) => panic!("model was dropped"),
Err(_) => {
if !cx.executor().tick() {
break;
}
}
}
}
panic!("no event received")
}
}

66
crates/gpui2/src/executor.rs
View File

@ -5,10 +5,11 @@ use std::{
fmt::Debug,
marker::PhantomData,
mem,
num::NonZeroUsize,
pin::Pin,
rc::Rc,
sync::{
atomic::{AtomicBool, Ordering::SeqCst},
atomic::{AtomicBool, AtomicUsize, Ordering::SeqCst},
Arc,
},
task::{Context, Poll},
@ -71,30 +72,57 @@ impl<T> Future for Task<T> {
}
}
}
#[derive(Clone, Copy, PartialEq, Eq, Hash, Debug)]
pub struct TaskLabel(NonZeroUsize);
impl TaskLabel {
pub fn new() -> Self {
static NEXT_TASK_LABEL: AtomicUsize = AtomicUsize::new(1);
Self(NEXT_TASK_LABEL.fetch_add(1, SeqCst).try_into().unwrap())
}
}
type AnyLocalFuture<R> = Pin<Box<dyn 'static + Future<Output = R>>>;
type AnyFuture<R> = Pin<Box<dyn 'static + Send + Future<Output = R>>>;
impl BackgroundExecutor {
pub fn new(dispatcher: Arc<dyn PlatformDispatcher>) -> Self {
Self { dispatcher }
}
/// Enqueues the given closure to be run on any thread. The closure returns
/// a future which will be run to completion on any available thread.
/// Enqueues the given future to be run to completion on a background thread.
pub fn spawn<R>(&self, future: impl Future<Output = R> + Send + 'static) -> Task<R>
where
R: Send + 'static,
{
self.spawn_internal::<R>(Box::pin(future), None)
}
/// Enqueues the given future to be run to completion on a background thread.
/// The given label can be used to control the priority of the task in tests.
pub fn spawn_labeled<R>(
&self,
label: TaskLabel,
future: impl Future<Output = R> + Send + 'static,
) -> Task<R>
where
R: Send + 'static,
{
self.spawn_internal::<R>(Box::pin(future), Some(label))
}
fn spawn_internal<R: Send + 'static>(
&self,
future: AnyFuture<R>,
label: Option<TaskLabel>,
) -> Task<R> {
let dispatcher = self.dispatcher.clone();
fn inner<R: Send + 'static>(
dispatcher: Arc<dyn PlatformDispatcher>,
future: AnyFuture<R>,
) -> Task<R> {
let (runnable, task) =
async_task::spawn(future, move |runnable| dispatcher.dispatch(runnable));
runnable.schedule();
Task::Spawned(task)
}
inner::<R>(dispatcher, Box::pin(future))
let (runnable, task) =
async_task::spawn(future, move |runnable| dispatcher.dispatch(runnable, label));
runnable.schedule();
Task::Spawned(task)
}
#[cfg(any(test, feature = "test-support"))]
@ -130,7 +158,7 @@ impl BackgroundExecutor {
match future.as_mut().poll(&mut cx) {
Poll::Ready(result) => return result,
Poll::Pending => {
if !self.dispatcher.poll(background_only) {
if !self.dispatcher.tick(background_only) {
if awoken.swap(false, SeqCst) {
continue;
}
@ -216,11 +244,21 @@ impl BackgroundExecutor {
self.dispatcher.as_test().unwrap().simulate_random_delay()
}
#[cfg(any(test, feature = "test-support"))]
pub fn deprioritize(&self, task_label: TaskLabel) {
self.dispatcher.as_test().unwrap().deprioritize(task_label)
}
#[cfg(any(test, feature = "test-support"))]
pub fn advance_clock(&self, duration: Duration) {
self.dispatcher.as_test().unwrap().advance_clock(duration)
}
#[cfg(any(test, feature = "test-support"))]
pub fn tick(&self) -> bool {
self.dispatcher.as_test().unwrap().tick(false)
}
#[cfg(any(test, feature = "test-support"))]
pub fn run_until_parked(&self) {
self.dispatcher.as_test().unwrap().run_until_parked()

6
crates/gpui2/src/platform.rs
View File

@ -8,7 +8,7 @@ use crate::{
point, size, AnyWindowHandle, BackgroundExecutor, Bounds, DevicePixels, Font, FontId,
FontMetrics, FontRun, ForegroundExecutor, GlobalPixels, GlyphId, InputEvent, LineLayout,
Pixels, Point, RenderGlyphParams, RenderImageParams, RenderSvgParams, Result, Scene,
SharedString, Size,
SharedString, Size, TaskLabel,
};
use anyhow::{anyhow, bail};
use async_task::Runnable;
@ -162,10 +162,10 @@ pub(crate) trait PlatformWindow {
pub trait PlatformDispatcher: Send + Sync {
fn is_main_thread(&self) -> bool;
fn dispatch(&self, runnable: Runnable);
fn dispatch(&self, runnable: Runnable, label: Option<TaskLabel>);
fn dispatch_on_main_thread(&self, runnable: Runnable);
fn dispatch_after(&self, duration: Duration, runnable: Runnable);
fn poll(&self, background_only: bool) -> bool;
fn tick(&self, background_only: bool) -> bool;
fn park(&self);
fn unparker(&self) -> Unparker;

6
crates/gpui2/src/platform/mac/dispatcher.rs
View File

@ -2,7 +2,7 @@
#![allow(non_camel_case_types)]
#![allow(non_snake_case)]
use crate::PlatformDispatcher;
use crate::{PlatformDispatcher, TaskLabel};
use async_task::Runnable;
use objc::{
class, msg_send,
@ -37,7 +37,7 @@ impl PlatformDispatcher for MacDispatcher {
is_main_thread == YES
}
fn dispatch(&self, runnable: Runnable) {
fn dispatch(&self, runnable: Runnable, _: Option<TaskLabel>) {
unsafe {
dispatch_async_f(
dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT.try_into().unwrap(), 0),
@ -71,7 +71,7 @@ impl PlatformDispatcher for MacDispatcher {
}
}
fn poll(&self, _background_only: bool) -> bool {
fn tick(&self, _background_only: bool) -> bool {
false
}

83
crates/gpui2/src/platform/test/dispatcher.rs
View File

@ -1,7 +1,7 @@
use crate::PlatformDispatcher;
use crate::{PlatformDispatcher, TaskLabel};
use async_task::Runnable;
use backtrace::Backtrace;
use collections::{HashMap, VecDeque};
use collections::{HashMap, HashSet, VecDeque};
use parking::{Parker, Unparker};
use parking_lot::Mutex;
use rand::prelude::*;
@ -28,12 +28,14 @@ struct TestDispatcherState {
random: StdRng,
foreground: HashMap<TestDispatcherId, VecDeque<Runnable>>,
background: Vec<Runnable>,
deprioritized_background: Vec<Runnable>,
delayed: Vec<(Duration, Runnable)>,
time: Duration,
is_main_thread: bool,
next_id: TestDispatcherId,
allow_parking: bool,
waiting_backtrace: Option<Backtrace>,
deprioritized_task_labels: HashSet<TaskLabel>,
}
impl TestDispatcher {
@ -43,12 +45,14 @@ impl TestDispatcher {
random,
foreground: HashMap::default(),
background: Vec::new(),
deprioritized_background: Vec::new(),
delayed: Vec::new(),
time: Duration::ZERO,
is_main_thread: true,
next_id: TestDispatcherId(1),
allow_parking: false,
waiting_backtrace: None,
deprioritized_task_labels: Default::default(),
};
TestDispatcher {
@ -101,8 +105,15 @@ impl TestDispatcher {
}
}
pub fn deprioritize(&self, task_label: TaskLabel) {
self.state
.lock()
.deprioritized_task_labels
.insert(task_label);
}
pub fn run_until_parked(&self) {
while self.poll(false) {}
while self.tick(false) {}
}
pub fn parking_allowed(&self) -> bool {
@ -150,8 +161,17 @@ impl PlatformDispatcher for TestDispatcher {
self.state.lock().is_main_thread
}
fn dispatch(&self, runnable: Runnable) {
self.state.lock().background.push(runnable);
fn dispatch(&self, runnable: Runnable, label: Option<TaskLabel>) {
{
let mut state = self.state.lock();
if label.map_or(false, |label| {
state.deprioritized_task_labels.contains(&label)
}) {
state.deprioritized_background.push(runnable);
} else {
state.background.push(runnable);
}
}
self.unparker.unpark();
}
@ -174,7 +194,7 @@ impl PlatformDispatcher for TestDispatcher {
state.delayed.insert(ix, (next_time, runnable));
}
fn poll(&self, background_only: bool) -> bool {
fn tick(&self, background_only: bool) -> bool {
let mut state = self.state.lock();
while let Some((deadline, _)) = state.delayed.first() {
@ -196,34 +216,41 @@ impl PlatformDispatcher for TestDispatcher {
};
let background_len = state.background.len();
let runnable;
let main_thread;
if foreground_len == 0 && background_len == 0 {
return false;
}
let main_thread = state.random.gen_ratio(
foreground_len as u32,
(foreground_len + background_len) as u32,
);
let was_main_thread = state.is_main_thread;
state.is_main_thread = main_thread;
let runnable = if main_thread {
let state = &mut *state;
let runnables = state
.foreground
.values_mut()
.filter(|runnables| !runnables.is_empty())
.choose(&mut state.random)
.unwrap();
runnables.pop_front().unwrap()
let deprioritized_background_len = state.deprioritized_background.len();
if deprioritized_background_len == 0 {
return false;
}
let ix = state.random.gen_range(0..deprioritized_background_len);
main_thread = false;
runnable = state.deprioritized_background.swap_remove(ix);
} else {
let ix = state.random.gen_range(0..background_len);
state.background.swap_remove(ix)
main_thread = state.random.gen_ratio(
foreground_len as u32,
(foreground_len + background_len) as u32,
);
if main_thread {
let state = &mut *state;
runnable = state
.foreground
.values_mut()
.filter(|runnables| !runnables.is_empty())
.choose(&mut state.random)
.unwrap()
.pop_front()
.unwrap();
} else {
let ix = state.random.gen_range(0..background_len);
runnable = state.background.swap_remove(ix);
};
};
let was_main_thread = state.is_main_thread;
state.is_main_thread = main_thread;
drop(state);
runnable.run();
self.state.lock().is_main_thread = was_main_thread;
true

90
crates/language/src/buffer.rs
View File

@ -17,7 +17,7 @@ use crate::{
};
use anyhow::{anyhow, Result};
pub use clock::ReplicaId;
use futures::FutureExt as _;
use futures::channel::oneshot;
use gpui::{fonts::HighlightStyle, AppContext, Entity, ModelContext, Task};
use lsp::LanguageServerId;
use parking_lot::Mutex;
@ -45,7 +45,7 @@ pub use text::{Buffer as TextBuffer, BufferSnapshot as TextBufferSnapshot, *};
use theme::SyntaxTheme;
#[cfg(any(test, feature = "test-support"))]
use util::RandomCharIter;
use util::{RangeExt, TryFutureExt as _};
use util::RangeExt;
#[cfg(any(test, feature = "test-support"))]
pub use {tree_sitter_rust, tree_sitter_typescript};
@ -62,6 +62,7 @@ pub struct Buffer {
saved_mtime: SystemTime,
transaction_depth: usize,
was_dirty_before_starting_transaction: Option<bool>,
reload_task: Option<Task<Result<()>>>,
language: Option<Arc<Language>>,
autoindent_requests: Vec<Arc<AutoindentRequest>>,
pending_autoindent: Option<Task<()>>,
@ -509,6 +510,7 @@ impl Buffer {
saved_mtime,
saved_version: buffer.version(),
saved_version_fingerprint: buffer.as_rope().fingerprint(),
reload_task: None,
transaction_depth: 0,
was_dirty_before_starting_transaction: None,
text: buffer,
@ -608,37 +610,52 @@ impl Buffer {
cx.notify();
}
pub fn reload(&mut self, cx: &mut ModelContext<Self>) -> Task<Result<Option<Transaction>>> {
cx.spawn(|this, mut cx| async move {
if let Some((new_mtime, new_text)) = this.read_with(&cx, |this, cx| {
pub fn reload(
&mut self,
cx: &mut ModelContext<Self>,
) -> oneshot::Receiver<Option<Transaction>> {
let (tx, rx) = futures::channel::oneshot::channel();
let prev_version = self.text.version();
self.reload_task = Some(cx.spawn(|this, mut cx| async move {
let Some((new_mtime, new_text)) = this.update(&mut cx, |this, cx| {
let file = this.file.as_ref()?.as_local()?;
Some((file.mtime(), file.load(cx)))
}) {
let new_text = new_text.await?;
let diff = this
.read_with(&cx, |this, cx| this.diff(new_text, cx))
.await;
this.update(&mut cx, |this, cx| {
if this.version() == diff.base_version {
this.finalize_last_transaction();
this.apply_diff(diff, cx);
if let Some(transaction) = this.finalize_last_transaction().cloned() {
this.did_reload(
this.version(),
this.as_rope().fingerprint(),
this.line_ending(),
new_mtime,
cx,
);
return Ok(Some(transaction));
}
}
Ok(None)
})
} else {
Ok(None)
}
})
}) else {
return Ok(());
};
let new_text = new_text.await?;
let diff = this
.update(&mut cx, |this, cx| this.diff(new_text.clone(), cx))
.await;
this.update(&mut cx, |this, cx| {
if this.version() == diff.base_version {
this.finalize_last_transaction();
this.apply_diff(diff, cx);
tx.send(this.finalize_last_transaction().cloned()).ok();
this.did_reload(
this.version(),
this.as_rope().fingerprint(),
this.line_ending(),
new_mtime,
cx,
);
} else {
this.did_reload(
prev_version,
Rope::text_fingerprint(&new_text),
this.line_ending(),
this.saved_mtime,
cx,
);
}
this.reload_task.take();
});
Ok(())
}));
rx
}
pub fn did_reload(
@ -667,13 +684,8 @@ impl Buffer {
cx.notify();
}
pub fn file_updated(
&mut self,
new_file: Arc<dyn File>,
cx: &mut ModelContext<Self>,
) -> Task<()> {
pub fn file_updated(&mut self, new_file: Arc<dyn File>, cx: &mut ModelContext<Self>) {
let mut file_changed = false;
let mut task = Task::ready(());
if let Some(old_file) = self.file.as_ref() {
if new_file.path() != old_file.path() {
@ -693,8 +705,7 @@ impl Buffer {
file_changed = true;
if !self.is_dirty() {
let reload = self.reload(cx).log_err().map(drop);
task = cx.foreground().spawn(reload);
self.reload(cx).close();
}
}
}
@ -708,7 +719,6 @@ impl Buffer {
cx.emit(Event::FileHandleChanged);
cx.notify();
}
task
}
pub fn diff_base(&self) -> Option<&str> {

215
crates/language2/src/buffer.rs
View File

@ -16,8 +16,9 @@ use crate::{
};
use anyhow::{anyhow, Result};
pub use clock::ReplicaId;
use futures::FutureExt as _;
use gpui::{AppContext, EventEmitter, HighlightStyle, ModelContext, Task};
use futures::channel::oneshot;
use gpui::{AppContext, EventEmitter, HighlightStyle, ModelContext, Task, TaskLabel};
use lazy_static::lazy_static;
use lsp::LanguageServerId;
use parking_lot::Mutex;
use similar::{ChangeTag, TextDiff};
@ -44,23 +45,33 @@ pub use text::{Buffer as TextBuffer, BufferSnapshot as TextBufferSnapshot, *};
use theme::SyntaxTheme;
#[cfg(any(test, feature = "test-support"))]
use util::RandomCharIter;
use util::{RangeExt, TryFutureExt as _};
use util::RangeExt;
#[cfg(any(test, feature = "test-support"))]
pub use {tree_sitter_rust, tree_sitter_typescript};
pub use lsp::DiagnosticSeverity;
lazy_static! {
pub static ref BUFFER_DIFF_TASK: TaskLabel = TaskLabel::new();
}
pub struct Buffer {
text: TextBuffer,
diff_base: Option<String>,
git_diff: git::diff::BufferDiff,
file: Option<Arc<dyn File>>,
saved_version: clock::Global,
saved_version_fingerprint: RopeFingerprint,
/// The mtime of the file when this buffer was last loaded from
/// or saved to disk.
saved_mtime: SystemTime,
/// The version vector when this buffer was last loaded from
/// or saved to disk.
saved_version: clock::Global,
/// A hash of the current contents of the buffer's file.
file_fingerprint: RopeFingerprint,
transaction_depth: usize,
was_dirty_before_starting_transaction: Option<bool>,
reload_task: Option<Task<Result<()>>>,
language: Option<Arc<Language>>,
autoindent_requests: Vec<Arc<AutoindentRequest>>,
pending_autoindent: Option<Task<()>>,
@ -380,8 +391,7 @@ impl Buffer {
.ok_or_else(|| anyhow!("missing line_ending"))?,
));
this.saved_version = proto::deserialize_version(&message.saved_version);
this.saved_version_fingerprint =
proto::deserialize_fingerprint(&message.saved_version_fingerprint)?;
this.file_fingerprint = proto::deserialize_fingerprint(&message.saved_version_fingerprint)?;
this.saved_mtime = message
.saved_mtime
.ok_or_else(|| anyhow!("invalid saved_mtime"))?
@ -397,7 +407,7 @@ impl Buffer {
diff_base: self.diff_base.as_ref().map(|h| h.to_string()),
line_ending: proto::serialize_line_ending(self.line_ending()) as i32,
saved_version: proto::serialize_version(&self.saved_version),
saved_version_fingerprint: proto::serialize_fingerprint(self.saved_version_fingerprint),
saved_version_fingerprint: proto::serialize_fingerprint(self.file_fingerprint),
saved_mtime: Some(self.saved_mtime.into()),
}
}
@ -467,7 +477,8 @@ impl Buffer {
Self {
saved_mtime,
saved_version: buffer.version(),
saved_version_fingerprint: buffer.as_rope().fingerprint(),
file_fingerprint: buffer.as_rope().fingerprint(),
reload_task: None,
transaction_depth: 0,
was_dirty_before_starting_transaction: None,
text: buffer,
@ -533,7 +544,7 @@ impl Buffer {
}
pub fn saved_version_fingerprint(&self) -> RopeFingerprint {
self.saved_version_fingerprint
self.file_fingerprint
}
pub fn saved_mtime(&self) -> SystemTime {
@ -561,43 +572,58 @@ impl Buffer {
cx: &mut ModelContext<Self>,
) {
self.saved_version = version;
self.saved_version_fingerprint = fingerprint;
self.file_fingerprint = fingerprint;
self.saved_mtime = mtime;
cx.emit(Event::Saved);
cx.notify();
}
pub fn reload(&mut self, cx: &mut ModelContext<Self>) -> Task<Result<Option<Transaction>>> {
cx.spawn(|this, mut cx| async move {
if let Some((new_mtime, new_text)) = this.update(&mut cx, |this, cx| {
pub fn reload(
&mut self,
cx: &mut ModelContext<Self>,
) -> oneshot::Receiver<Option<Transaction>> {
let (tx, rx) = futures::channel::oneshot::channel();
let prev_version = self.text.version();
self.reload_task = Some(cx.spawn(|this, mut cx| async move {
let Some((new_mtime, new_text)) = this.update(&mut cx, |this, cx| {
let file = this.file.as_ref()?.as_local()?;
Some((file.mtime(), file.load(cx)))
})? {
let new_text = new_text.await?;
let diff = this
.update(&mut cx, |this, cx| this.diff(new_text, cx))?
.await;
this.update(&mut cx, |this, cx| {
if this.version() == diff.base_version {
this.finalize_last_transaction();
this.apply_diff(diff, cx);
if let Some(transaction) = this.finalize_last_transaction().cloned() {
this.did_reload(
this.version(),
this.as_rope().fingerprint(),
this.line_ending(),
new_mtime,
cx,
);
return Some(transaction);
}
}
None
})
} else {
Ok(None)
}
})
})?
else {
return Ok(());
};
let new_text = new_text.await?;
let diff = this
.update(&mut cx, |this, cx| this.diff(new_text.clone(), cx))?
.await;
this.update(&mut cx, |this, cx| {
if this.version() == diff.base_version {
this.finalize_last_transaction();
this.apply_diff(diff, cx);
tx.send(this.finalize_last_transaction().cloned()).ok();
this.did_reload(
this.version(),
this.as_rope().fingerprint(),
this.line_ending(),
new_mtime,
cx,
);
} else {
this.did_reload(
prev_version,
Rope::text_fingerprint(&new_text),
this.line_ending(),
this.saved_mtime,
cx,
);
}
this.reload_task.take();
})
}));
rx
}
pub fn did_reload(
@ -609,14 +635,14 @@ impl Buffer {
cx: &mut ModelContext<Self>,
) {
self.saved_version = version;
self.saved_version_fingerprint = fingerprint;
self.file_fingerprint = fingerprint;
self.text.set_line_ending(line_ending);
self.saved_mtime = mtime;
if let Some(file) = self.file.as_ref().and_then(|f| f.as_local()) {
file.buffer_reloaded(
self.remote_id(),
&self.saved_version,
self.saved_version_fingerprint,
self.file_fingerprint,
self.line_ending(),
self.saved_mtime,
cx,
@ -626,13 +652,8 @@ impl Buffer {
cx.notify();
}
pub fn file_updated(
&mut self,
new_file: Arc<dyn File>,
cx: &mut ModelContext<Self>,
) -> Task<()> {
pub fn file_updated(&mut self, new_file: Arc<dyn File>, cx: &mut ModelContext<Self>) {
let mut file_changed = false;
let mut task = Task::ready(());
if let Some(old_file) = self.file.as_ref() {
if new_file.path() != old_file.path() {
@ -652,8 +673,7 @@ impl Buffer {
file_changed = true;
if !self.is_dirty() {
let reload = self.reload(cx).log_err().map(drop);
task = cx.background_executor().spawn(reload);
self.reload(cx).close();
}
}
}
@ -667,7 +687,6 @@ impl Buffer {
cx.emit(Event::FileHandleChanged);
cx.notify();
}
task
}
pub fn diff_base(&self) -> Option<&str> {
@ -1118,36 +1137,72 @@ impl Buffer {
pub fn diff(&self, mut new_text: String, cx: &AppContext) -> Task<Diff> {
let old_text = self.as_rope().clone();
let base_version = self.version();
cx.background_executor().spawn(async move {
let old_text = old_text.to_string();
let line_ending = LineEnding::detect(&new_text);
LineEnding::normalize(&mut new_text);
let diff = TextDiff::from_chars(old_text.as_str(), new_text.as_str());
let mut edits = Vec::new();
let mut offset = 0;
let empty: Arc<str> = "".into();
for change in diff.iter_all_changes() {
let value = change.value();
let end_offset = offset + value.len();
match change.tag() {
ChangeTag::Equal => {
offset = end_offset;
cx.background_executor()
.spawn_labeled(*BUFFER_DIFF_TASK, async move {
let old_text = old_text.to_string();
let line_ending = LineEnding::detect(&new_text);
LineEnding::normalize(&mut new_text);
let diff = TextDiff::from_chars(old_text.as_str(), new_text.as_str());
let empty: Arc<str> = "".into();
let mut edits = Vec::new();
let mut old_offset = 0;
let mut new_offset = 0;
let mut last_edit: Option<(Range<usize>, Range<usize>)> = None;
for change in diff.iter_all_changes().map(Some).chain([None]) {
if let Some(change) = &change {
let len = change.value().len();
match change.tag() {
ChangeTag::Equal => {
old_offset += len;
new_offset += len;
}
ChangeTag::Delete => {
let old_end_offset = old_offset + len;
if let Some((last_old_range, _)) = &mut last_edit {
last_old_range.end = old_end_offset;
} else {
last_edit =
Some((old_offset..old_end_offset, new_offset..new_offset));
}
old_offset = old_end_offset;
}
ChangeTag::Insert => {
let new_end_offset = new_offset + len;
if let Some((_, last_new_range)) = &mut last_edit {
last_new_range.end = new_end_offset;
} else {
last_edit =
Some((old_offset..old_offset, new_offset..new_end_offset));
}
new_offset = new_end_offset;
}
}
}
ChangeTag::Delete => {
edits.push((offset..end_offset, empty.clone()));
offset = end_offset;
}
ChangeTag::Insert => {
edits.push((offset..offset, value.into()));
if let Some((old_range, new_range)) = &last_edit {
if old_offset > old_range.end
|| new_offset > new_range.end
|| change.is_none()
{
let text = if new_range.is_empty() {
empty.clone()
} else {
new_text[new_range.clone()].into()
};
edits.push((old_range.clone(), text));
last_edit.take();
}
}
}
}
Diff {
base_version,
line_ending,
edits,
}
})
Diff {
base_version,
line_ending,
edits,
}
})
}
/// Spawn a background task that searches the buffer for any whitespace
@ -1231,12 +1286,12 @@ impl Buffer {
}
pub fn is_dirty(&self) -> bool {
self.saved_version_fingerprint != self.as_rope().fingerprint()
self.file_fingerprint != self.as_rope().fingerprint()
|| self.file.as_ref().map_or(false, |file| file.is_deleted())
}
pub fn has_conflict(&self) -> bool {
self.saved_version_fingerprint != self.as_rope().fingerprint()
self.file_fingerprint != self.as_rope().fingerprint()
&& self
.file
.as_ref()

4
crates/project/src/project.rs
View File

@ -6190,7 +6190,7 @@ impl Project {
.log_err();
}
buffer.file_updated(Arc::new(new_file), cx).detach();
buffer.file_updated(Arc::new(new_file), cx);
}
}
});
@ -7182,7 +7182,7 @@ impl Project {
.ok_or_else(|| anyhow!("no such worktree"))?;
let file = File::from_proto(file, worktree, cx)?;
buffer.update(cx, |buffer, cx| {
buffer.file_updated(Arc::new(file), cx).detach();
buffer.file_updated(Arc::new(file), cx);
});
this.detect_language_for_buffer(&buffer, cx);
}

2
crates/project/src/worktree.rs
View File

@ -959,7 +959,7 @@ impl LocalWorktree {
buffer_handle.update(&mut cx, |buffer, cx| {
if has_changed_file {
buffer.file_updated(new_file, cx).detach();
buffer.file_updated(new_file, cx);
}
});
}

4
crates/project2/src/project2.rs
View File

@ -6262,7 +6262,7 @@ impl Project {
.log_err();
}
buffer.file_updated(Arc::new(new_file), cx).detach();
buffer.file_updated(Arc::new(new_file), cx);
}
}
});
@ -7256,7 +7256,7 @@ impl Project {
.ok_or_else(|| anyhow!("no such worktree"))?;
let file = File::from_proto(file, worktree, cx)?;
buffer.update(cx, |buffer, cx| {
buffer.file_updated(Arc::new(file), cx).detach();
buffer.file_updated(Arc::new(file), cx);
});
this.detect_language_for_buffer(&buffer, cx);
}

67
crates/project2/src/project_tests.rs
View File

@ -2587,6 +2587,73 @@ async fn test_save_file(cx: &mut gpui::TestAppContext) {
assert_eq!(new_text, buffer.update(cx, |buffer, _| buffer.text()));
}
#[gpui::test(iterations = 30)]
async fn test_file_changes_multiple_times_on_disk(cx: &mut gpui::TestAppContext) {
init_test(cx);
let fs = FakeFs::new(cx.executor().clone());
fs.insert_tree(
"/dir",
json!({
"file1": "the original contents",
}),
)
.await;
let project = Project::test(fs.clone(), ["/dir".as_ref()], cx).await;
let worktree = project.read_with(cx, |project, _| project.worktrees().next().unwrap());
let buffer = project
.update(cx, |p, cx| p.open_local_buffer("/dir/file1", cx))
.await
.unwrap();
// Simulate buffer diffs being slow, so that they don't complete before
// the next file change occurs.
cx.executor().deprioritize(*language::BUFFER_DIFF_TASK);
// Change the buffer's file on disk, and then wait for the file change
// to be detected by the worktree, so that the buffer starts reloading.
fs.save(
"/dir/file1".as_ref(),
&"the first contents".into(),
Default::default(),
)
.await
.unwrap();
worktree.next_event(cx);
// Change the buffer's file again. Depending on the random seed, the
// previous file change may still be in progress.
fs.save(
"/dir/file1".as_ref(),
&"the second contents".into(),
Default::default(),
)
.await
.unwrap();
worktree.next_event(cx);
cx.executor().run_until_parked();
let on_disk_text = fs.load(Path::new("/dir/file1")).await.unwrap();
buffer.read_with(cx, |buffer, _| {
let buffer_text = buffer.text();
if buffer_text == on_disk_text {
assert!(
!buffer.is_dirty() && !buffer.has_conflict(),
"buffer shouldn't be dirty. text: {buffer_text:?}, disk text: {on_disk_text:?}",
);
}
// If the file change occurred while the buffer was processing the first
// change, the buffer will be in a conflicting state.
else {
assert!(
buffer.is_dirty() && buffer.has_conflict(),
"buffer should report that it has a conflict. text: {buffer_text:?}, disk text: {on_disk_text:?}"
);
}
});
}
#[gpui::test]
async fn test_save_in_single_file_worktree(cx: &mut gpui::TestAppContext) {
init_test(cx);

3
crates/project2/src/worktree.rs
View File

@ -276,6 +276,7 @@ struct ShareState {
_maintain_remote_snapshot: Task<Option<()>>,
}
#[derive(Clone)]
pub enum Event {
UpdatedEntries(UpdatedEntriesSet),
UpdatedGitRepositories(UpdatedGitRepositoriesSet),
@ -961,7 +962,7 @@ impl LocalWorktree {
buffer_handle.update(&mut cx, |buffer, cx| {
if has_changed_file {
buffer.file_updated(new_file, cx).detach();
buffer.file_updated(new_file, cx);
}
})?;
}

6
crates/rope/src/rope.rs
View File

@ -41,6 +41,10 @@ impl Rope {
Self::default()
}
pub fn text_fingerprint(text: &str) -> RopeFingerprint {
bromberg_sl2::hash_strict(text.as_bytes())
}
pub fn append(&mut self, rope: Rope) {
let mut chunks = rope.chunks.cursor::<()>();
chunks.next(&());
@ -931,7 +935,7 @@ impl<'a> From<&'a str> for ChunkSummary {
fn from(text: &'a str) -> Self {
Self {
text: TextSummary::from(text),
fingerprint: bromberg_sl2::hash_strict(text.as_bytes()),
fingerprint: Rope::text_fingerprint(text),
}
}
}

6
crates/rope2/src/rope2.rs
View File

@ -41,6 +41,10 @@ impl Rope {
Self::default()
}
pub fn text_fingerprint(text: &str) -> RopeFingerprint {
bromberg_sl2::hash_strict(text.as_bytes())
}
pub fn append(&mut self, rope: Rope) {
let mut chunks = rope.chunks.cursor::<()>();
chunks.next(&());
@ -931,7 +935,7 @@ impl<'a> From<&'a str> for ChunkSummary {
fn from(text: &'a str) -> Self {
Self {
text: TextSummary::from(text),
fingerprint: bromberg_sl2::hash_strict(text.as_bytes()),
fingerprint: Rope::text_fingerprint(text),
}
}
}