Skip to content

Commit eee733d

Browse files
Ensure documents are synced before calculating completions
1 parent f53a932 commit eee733d

File tree

2 files changed

+60
-4
lines changed

2 files changed

+60
-4
lines changed

compiler/crates/relay-lsp/src/server/mod.rs

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,7 +152,7 @@ where
152152
);
153153

154154
let task_processor = LSPTaskProcessor;
155-
let task_queue = TaskQueue::new(Arc::new(task_processor));
155+
let mut task_queue = TaskQueue::new(Arc::new(task_processor));
156156
let task_scheduler = task_queue.get_scheduler();
157157

158158
config.artifact_writer = Box::new(NoopArtifactWriter);
@@ -190,6 +190,12 @@ fn next_task(
190190
}
191191
}
192192

193+
static NOTIFCATIONS_MUTATING_LSP_STATE: [&str; 3] = [
194+
"textDocument/didOpen",
195+
"textDocument/didChange",
196+
"textDocument/didClose",
197+
];
198+
193199
struct LSPTaskProcessor;
194200

195201
impl<TPerfLogger: PerfLogger + 'static, TSchemaDocumentation: SchemaDocumentation + 'static>
@@ -209,6 +215,15 @@ impl<TPerfLogger: PerfLogger + 'static, TSchemaDocumentation: SchemaDocumentatio
209215
}
210216
}
211217
}
218+
219+
fn is_serial_task(&self, task: &Task) -> bool {
220+
match task {
221+
Task::InboundMessage(Message::Notification(notification)) => {
222+
NOTIFCATIONS_MUTATING_LSP_STATE.contains(&notification.method.as_str())
223+
}
224+
_ => false,
225+
}
226+
}
212227
}
213228

214229
fn handle_request<TPerfLogger: PerfLogger + 'static, TSchemaDocumentation: SchemaDocumentation>(

compiler/crates/relay-lsp/src/server/task_queue.rs

Lines changed: 44 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
use std::sync::Arc;
99
use std::thread;
10+
use std::thread::JoinHandle;
1011
use std::time::Instant;
1112

1213
use crossbeam::channel::unbounded;
@@ -18,10 +19,13 @@ pub struct TaskQueue<S, T> {
1819
processor: Arc<dyn TaskProcessor<S, T>>,
1920
pub receiver: Receiver<T>,
2021
scheduler: Arc<TaskScheduler<T>>,
22+
active_thread_handles: Vec<JoinHandle<()>>,
2123
}
2224

2325
pub trait TaskProcessor<S, T>: Send + Sync + 'static {
2426
fn process(&self, state: Arc<S>, task: T);
27+
28+
fn is_serial_task(&self, task: &T) -> bool;
2529
}
2630

2731
pub struct TaskScheduler<T> {
@@ -48,25 +52,62 @@ where
4852
processor,
4953
receiver,
5054
scheduler: Arc::new(TaskScheduler { sender }),
55+
active_thread_handles: Vec::new(),
5156
}
5257
}
5358

5459
pub fn get_scheduler(&self) -> Arc<TaskScheduler<T>> {
5560
Arc::clone(&self.scheduler)
5661
}
5762

58-
pub fn process(&self, state: Arc<S>, task: T) {
63+
pub fn process(&mut self, state: Arc<S>, task: T) {
64+
let processor = Arc::clone(&self.processor);
65+
let is_serial_task = processor.is_serial_task(&task);
66+
67+
if is_serial_task {
68+
// Before starting a serial task, we need to make sure that all
69+
// previous tasks have been completed, otherwise the serial task
70+
// might interfere with them.
71+
self.ensure_previous_tasks_completed();
72+
} else {
73+
// We do this in order for the active threads to not grow
74+
// indefinitely, if there hasn't been a serial task in a while.
75+
self.cleanup_already_finished_tasks();
76+
}
77+
5978
let task_str = format!("{:?}", &task);
6079
let now = Instant::now();
6180
debug!("Processing task {:?}", &task_str);
62-
let processor = Arc::clone(&self.processor);
63-
thread::spawn(move || {
81+
82+
let handle = thread::spawn(move || {
6483
processor.process(state, task);
84+
6585
debug!(
6686
"task {} completed in {}ms",
6787
task_str,
6888
now.elapsed().as_millis()
6989
);
7090
});
91+
92+
if is_serial_task {
93+
// If the task is serial, we need to wait for its thread
94+
// to complete, before moving onto the next task.
95+
let _ = handle.join();
96+
} else {
97+
self.active_thread_handles.push(handle);
98+
}
99+
}
100+
101+
fn ensure_previous_tasks_completed(&mut self) {
102+
for handle in self.active_thread_handles.drain(..) {
103+
// We don't actually care whether the thread has panicked or not,
104+
// we just want to make sure it's finished.
105+
let _ = handle.join();
106+
}
107+
}
108+
109+
fn cleanup_already_finished_tasks(&mut self) {
110+
self.active_thread_handles
111+
.retain(|handle| handle.is_finished() == false);
71112
}
72113
}

0 commit comments

Comments
 (0)