refactor: use a single Mutex in ProcState for module graph #12489
Conversation
cli/proc_state.rs
Outdated
| @@ -52,31 +52,35 @@ use std::sync::Arc; | |||
| #[derive(Clone)] | |||
| pub struct ProcState(Arc<Inner>); | |||
|
|
|||
| #[derive(Default)] | |||
| struct GraphStuff { | |||
There was a problem hiding this comment.
Better name suggestions welcome
There was a problem hiding this comment.
Yeah lol. Maybe SynchronizedGraphData then graph_data? Or maybe just GraphData.
There was a problem hiding this comment.
Renamed to GraphData
| // deno_graph detects all sorts of issues at build time (prepare_module_load) | ||
| // but if they are errors at that stage, the don't cause the correct behaviors | ||
| // so we cache the error and then surface it when appropriate (e.g. load) | ||
| maybe_graph_error: Option<deno_graph::ModuleGraphError>, |
There was a problem hiding this comment.
I believe this is wrong - if there are multiple threads (eg. two web workers) loading code, then this value might be overwritten by accident. I think we should store this struct per worker, so move it to CliModuleLoader
There was a problem hiding this comment.
Directly related to #12458
EDIT: Looks like Andreu was faster :P
cli/proc_state.rs
Outdated
| } | ||
|
|
||
| // If we're this far it means that there was an error for this module load. | ||
| let mut graph_stuff = self.graph_stuff.lock(); |
There was a problem hiding this comment.
Lock is released and then immediately acquired. Is that intentional?
There was a problem hiding this comment.
What do you mean? This lock is valid until the body of the function
This commit factors out 4 different fields from "ProcState", that are behind
"Arc<Mutex<>>" into a single struct behind a single mutex.
Closes #12381