refactor: use of lock file in ModuleGraph2 (#8087)

cli/main.rs

@@ -182,9 +182,10 @@ async fn info_command(
     let mut builder = module_graph2::GraphBuilder2::new(
       handler,
       program_state.maybe_import_map.clone(),
+      program_state.lockfile.clone(),
     );
     builder.add(&specifier, false).await?;
-    let graph = builder.get_graph(&program_state.lockfile);
+    let graph = builder.get_graph();
     let info = graph.info()?;
 
     if json {
@@ -321,9 +322,10 @@ async fn bundle_command(
   let mut builder = module_graph2::GraphBuilder2::new(
     handler,
     program_state.maybe_import_map.clone(),
+    program_state.lockfile.clone(),
   );
   builder.add(&module_specifier, false).await?;
-  let graph = builder.get_graph(&program_state.lockfile);
+  let graph = builder.get_graph();
 
   let debug = flags.log_level == Some(log::Level::Debug);
   if !flags.no_check {

cli/module_graph2.rs

@@ -48,6 +48,7 @@ use std::fmt;
 use std::path::PathBuf;
 use std::rc::Rc;
 use std::result;
+use std::sync::Arc;
 use std::sync::Mutex;
 use std::time::Instant;
 
@@ -598,6 +599,8 @@ pub struct Graph2 {
   /// calls to a module graph where the emit is already valid do not cause the
   /// graph to re-emit.
   roots_dynamic: bool,
+  // A reference to lock file that will be used to check module integrity.
+  maybe_lockfile: Option<Arc<Mutex<Lockfile>>>,
 }
 
 impl Graph2 {
@@ -606,7 +609,10 @@ impl Graph2 {
   /// The argument `handler` is an instance of a structure that implements the
   /// `SpecifierHandler` trait.
   ///
-  pub fn new(handler: Rc<RefCell<dyn SpecifierHandler>>) -> Self {
+  pub fn new(
+    handler: Rc<RefCell<dyn SpecifierHandler>>,
+    maybe_lockfile: Option<Arc<Mutex<Lockfile>>>,
+  ) -> Self {
     Graph2 {
       handler,
       maybe_tsbuildinfo: None,
@@ -614,6 +620,7 @@ impl Graph2 {
       redirects: HashMap::new(),
       roots: Vec::new(),
       roots_dynamic: true,
+      maybe_lockfile,
     }
   }
 
@@ -1026,8 +1033,8 @@ impl Graph2 {
   /// Verify the subresource integrity of the graph based upon the optional
   /// lockfile, updating the lockfile with any missing resources.  This will
   /// error if any of the resources do not match their lock status.
-  pub fn lock(&self, maybe_lockfile: &Option<Mutex<Lockfile>>) {
+  pub fn lock(&self) {
-    if let Some(lf) = maybe_lockfile {
+    if let Some(lf) = self.maybe_lockfile.as_ref() {
       let mut lockfile = lf.lock().unwrap();
       for (ms, module) in self.modules.iter() {
         let specifier = module.specifier.to_string();
@@ -1259,6 +1266,7 @@ impl GraphBuilder2 {
   pub fn new(
     handler: Rc<RefCell<dyn SpecifierHandler>>,
     maybe_import_map: Option<ImportMap>,
+    maybe_lockfile: Option<Arc<Mutex<Lockfile>>>,
   ) -> Self {
     let internal_import_map = if let Some(import_map) = maybe_import_map {
       Some(Rc::new(RefCell::new(import_map)))
@@ -1266,7 +1274,7 @@ impl GraphBuilder2 {
       None
     };
     GraphBuilder2 {
-      graph: Graph2::new(handler),
+      graph: Graph2::new(handler, maybe_lockfile),
       fetched: HashSet::new(),
       maybe_import_map: internal_import_map,
       pending: FuturesUnordered::new(),
@@ -1393,13 +1401,8 @@ impl GraphBuilder2 {
   /// Move out the graph from the builder to be utilized further.  An optional
   /// lockfile can be provided, where if the sources in the graph do not match
   /// the expected lockfile, an error will be logged and the process will exit.
-  ///
+  pub fn get_graph(self) -> Graph2 {
-  /// TODO(@kitsonk) this should really be owned by the graph, but currently
+    self.graph.lock();
-  /// the lockfile is behind a mutex in program_state, which makes it really
-  /// hard to not pass around as a reference, which if the Graph owned it, it
-  /// would need lifetime parameters and lifetime parameters are 😭
-  pub fn get_graph(self, maybe_lockfile: &Option<Mutex<Lockfile>>) -> Graph2 {
-    self.graph.lock(maybe_lockfile);
     self.graph
   }
 }
@@ -1537,13 +1540,13 @@ pub mod tests {
       fixtures,
       ..MockSpecifierHandler::default()
     }));
-    let mut builder = GraphBuilder2::new(handler.clone(), None);
+    let mut builder = GraphBuilder2::new(handler.clone(), None, None);
     builder
       .add(&specifier, false)
       .await
       .expect("module not inserted");
 
-    (builder.get_graph(&None), handler)
+    (builder.get_graph(), handler)
   }
 
   #[test]
@@ -1645,12 +1648,12 @@ pub mod tests {
         fixtures: fixtures.clone(),
         ..MockSpecifierHandler::default()
       }));
-      let mut builder = GraphBuilder2::new(handler.clone(), None);
+      let mut builder = GraphBuilder2::new(handler.clone(), None, None);
       builder
         .add(&specifier, false)
         .await
         .expect("module not inserted");
-      let graph = builder.get_graph(&None);
+      let graph = builder.get_graph();
       let (actual, stats, maybe_ignored_options) = graph
         .bundle(BundleOptions::default())
         .expect("could not bundle");
@@ -1739,7 +1742,7 @@ pub mod tests {
       fixtures,
       ..MockSpecifierHandler::default()
     }));
-    let mut builder = GraphBuilder2::new(handler.clone(), None);
+    let mut builder = GraphBuilder2::new(handler.clone(), None, None);
     builder
       .add(&specifier, false)
       .await
@@ -1845,12 +1848,12 @@ pub mod tests {
     let lockfile_path = fixtures.join("lockfile.json");
     let lockfile =
       Lockfile::new(lockfile_path, false).expect("could not load lockfile");
-    let maybe_lockfile = Some(Mutex::new(lockfile));
+    let maybe_lockfile = Some(Arc::new(Mutex::new(lockfile)));
     let handler = Rc::new(RefCell::new(MockSpecifierHandler {
       fixtures,
       ..MockSpecifierHandler::default()
     }));
-    let mut builder = GraphBuilder2::new(handler.clone(), None);
+    let mut builder = GraphBuilder2::new(handler.clone(), None, maybe_lockfile);
     let specifier =
       ModuleSpecifier::resolve_url_or_path("file:///tests/main.ts")
         .expect("could not resolve module");
@@ -1858,6 +1861,6 @@ pub mod tests {
       .add(&specifier, false)
       .await
       .expect("module not inserted");
-    builder.get_graph(&maybe_lockfile);
+    builder.get_graph();
   }
 }

cli/program_state.rs

@@ -48,7 +48,7 @@ pub struct ProgramState {
   pub dir: deno_dir::DenoDir,
   pub file_fetcher: SourceFileFetcher,
   pub ts_compiler: TsCompiler,
-  pub lockfile: Option<Mutex<Lockfile>>,
+  pub lockfile: Option<Arc<Mutex<Lockfile>>>,
   pub maybe_import_map: Option<ImportMap>,
   pub maybe_inspector_server: Option<Arc<InspectorServer>>,
 }
@@ -78,7 +78,7 @@ impl ProgramState {
 
     let lockfile = if let Some(filename) = &flags.lock {
       let lockfile = Lockfile::new(filename.clone(), flags.lock_write)?;
-      Some(Mutex::new(lockfile))
+      Some(Arc::new(Mutex::new(lockfile)))
     } else {
       None
     };
@@ -128,9 +128,10 @@ impl ProgramState {
     let specifier = specifier.clone();
     let handler =
       Rc::new(RefCell::new(FetchHandler::new(self, dynamic_permissions)?));
-    let mut builder = GraphBuilder2::new(handler, maybe_import_map);
+    let mut builder =
+      GraphBuilder2::new(handler, maybe_import_map, self.lockfile.clone());
     builder.add(&specifier, is_dynamic).await?;
-    let mut graph = builder.get_graph(&self.lockfile);
+    let mut graph = builder.get_graph();
     let debug = self.flags.log_level == Some(log::Level::Debug);
     let maybe_config_path = self.flags.config_path.clone();
 

cli/tsc2.rs

@@ -348,12 +348,12 @@ mod tests {
       fixtures,
       ..MockSpecifierHandler::default()
     }));
-    let mut builder = GraphBuilder2::new(handler.clone(), None);
+    let mut builder = GraphBuilder2::new(handler.clone(), None, None);
     builder
       .add(&specifier, false)
       .await
       .expect("module not inserted");
-    let graph = Rc::new(RefCell::new(builder.get_graph(&None)));
+    let graph = Rc::new(RefCell::new(builder.get_graph()));
     State::new(graph, hash_data, maybe_tsbuildinfo)
   }
 
@@ -572,12 +572,12 @@ mod tests {
       fixtures,
       ..MockSpecifierHandler::default()
     }));
-    let mut builder = GraphBuilder2::new(handler.clone(), None);
+    let mut builder = GraphBuilder2::new(handler.clone(), None, None);
     builder
       .add(&specifier, false)
       .await
       .expect("module not inserted");
-    let graph = Rc::new(RefCell::new(builder.get_graph(&None)));
+    let graph = Rc::new(RefCell::new(builder.get_graph()));
     let config = TsConfig::new(json!({
       "allowJs": true,
       "checkJs": false,