Skip to content

Commit 55c107d

Browse files
authored
fix: use correct Cell for EsmLibraryPlugin (#12067)
* fix: should clear oncell when rebuild * fix: use atomicRefCell to avoid mutation panic in watch mode * chore: explictly drop and write comments about the read lock
1 parent 3be48da commit 55c107d

File tree

5 files changed

+18
-14
lines changed

5 files changed

+18
-14
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/rspack_plugin_esm_library/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ version.workspace = true
1212

1313
[dependencies]
1414
async-trait = { workspace = true }
15+
atomic_refcell = { workspace = true }
1516
rayon = { workspace = true }
1617
regex = { workspace = true }
1718
rspack_cacheable = { workspace = true }

crates/rspack_plugin_esm_library/src/link.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -407,7 +407,8 @@ impl EsmLibraryPlugin {
407407
chunk_link.namespace_object_sources.insert(module, source);
408408
}
409409

410-
self.links.set(link).expect("should set chunk link");
410+
let mut links = self.links.borrow_mut();
411+
*links = link;
411412
Ok(())
412413
}
413414

crates/rspack_plugin_esm_library/src/plugin.rs

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ use std::{
33
sync::{Arc, LazyLock},
44
};
55

6+
use atomic_refcell::AtomicRefCell;
67
use regex::Regex;
78
use rspack_collections::{IdentifierIndexMap, IdentifierSet, UkeyMap};
89
use rspack_core::{
@@ -23,7 +24,7 @@ use rspack_plugin_javascript::{
2324
dependency::ImportDependencyTemplate, parser_and_generator::JavaScriptParserAndGenerator,
2425
};
2526
use sugar_path::SugarPath;
26-
use tokio::sync::{OnceCell, RwLock};
27+
use tokio::sync::RwLock;
2728

2829
use crate::{
2930
chunk_link::ChunkLinkContext, dependency::dyn_import::DynamicImportDependencyTemplate,
@@ -39,9 +40,10 @@ pub struct EsmLibraryPlugin {
3940
pub(crate) preserve_modules: Option<PathBuf>,
4041
// module instance will hold this map till compile done, we can't mutate it,
4142
// normal concatenateModule just read the info from it
42-
pub(crate) concatenated_modules_map_for_codegen: OnceCell<Arc<IdentifierIndexMap<ModuleInfo>>>,
43+
pub(crate) concatenated_modules_map_for_codegen:
44+
AtomicRefCell<Arc<IdentifierIndexMap<ModuleInfo>>>,
4345
pub(crate) concatenated_modules_map: RwLock<Arc<IdentifierIndexMap<ModuleInfo>>>,
44-
pub(crate) links: OnceCell<UkeyMap<ChunkUkey, ChunkLinkContext>>,
46+
pub(crate) links: AtomicRefCell<UkeyMap<ChunkUkey, ChunkLinkContext>>,
4547
}
4648

4749
impl EsmLibraryPlugin {
@@ -247,10 +249,9 @@ async fn finish_modules(&self, compilation: &mut Compilation) -> Result<()> {
247249

248250
// only used for scope
249251
// we mutably modify data in `self.concatenated_modules_map`
250-
self
251-
.concatenated_modules_map_for_codegen
252-
.set(Arc::new(modules_map.clone()))
253-
.expect("should set concatenated_modules_map_for_codegen");
252+
let mut map = self.concatenated_modules_map_for_codegen.borrow_mut();
253+
*map = Arc::new(modules_map.clone());
254+
drop(map);
254255

255256
*self.concatenated_modules_map.write().await = Arc::new(modules_map);
256257
// mark all entry exports as used
@@ -282,10 +283,7 @@ async fn concatenation_scope(
282283
_compilation: &Compilation,
283284
module: ModuleIdentifier,
284285
) -> Result<Option<ConcatenationScope>> {
285-
let modules_map = self
286-
.concatenated_modules_map_for_codegen
287-
.get()
288-
.expect("should already initialized");
286+
let modules_map = self.concatenated_modules_map_for_codegen.borrow();
289287

290288
let Some(current_module) = modules_map.get(&module) else {
291289
return Ok(None);

crates/rspack_plugin_esm_library/src/render.rs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,15 +76,18 @@ impl EsmLibraryPlugin {
7676
asset_info: &mut AssetInfo,
7777
) -> Result<Option<RenderSource>> {
7878
let module_graph = compilation.get_module_graph();
79-
let chunk_link_guard = self.links.get().expect("should have chunk link");
8079

81-
let chunk_link = chunk_link_guard.get(chunk_ukey).expect("should have chunk");
80+
// In this phase we only read from the lock, no write happen in this phase, the
81+
// next write happen only happen for next compile start
82+
let chunk_link_guard = self.links.borrow();
83+
let chunk_link = &chunk_link_guard[chunk_ukey];
8284

8385
let mut chunk_init_fragments: Vec<Box<dyn InitFragment<ChunkRenderContext> + 'static>> =
8486
chunk_link.init_fragments.clone();
8587
let mut replace_auto_public_path = false;
8688
let mut replace_static_url = false;
8789

90+
// Same as above, we can only read here, the write happen only at the finish_modules phase
8891
let concatenated_modules_map = self.concatenated_modules_map.read().await;
8992

9093
let chunk = get_chunk(compilation, *chunk_ukey);

0 commit comments

Comments
 (0)