Skip to content

Commit

Permalink
fix: should report errors if stats was being accessed after the compi…
Browse files Browse the repository at this point in the history
…ler was closed (#8561)
  • Loading branch information
h-a-n-a authored Nov 28, 2024
1 parent 5f77de0 commit d249e62
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 4 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@

let resolveCompilerStats;
let compilerStats = new Promise((resolve) => {
resolveCompilerStats = resolve
});

class MyPlugin {
apply(compiler) {
compiler.hooks.done.tap("Plugin", stats => {
resolveCompilerStats(stats)
});
}
}

/** @type {import('../../dist').TCompilerCaseConfig} */
module.exports = {
description: "should be called every compilation",
options(context) {
return {
context: context.getSource(),
entry: "./d",
plugins: [new MyPlugin()]
};
},
async build(_, compiler) {
await new Promise(resolve => {
compiler.run((err, stats) => {
compiler.close(() => {
// Should be able to access `Stats` within the same tick of closing.
expect(() => stats.compilation).not.toThrow();
resolve()
})
});
});
},
async check() {
let stats = await compilerStats;
// Should not be able to access `Stats` after the compiler was shutdown.
expect(() => stats.compilation).toThrow("Unable to access `Stats` after the compiler was shutdown")
}
};
5 changes: 4 additions & 1 deletion packages/rspack/etc/core.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -574,6 +574,9 @@ export class Compilation {
__internal__pushRspackDiagnostic(diagnostic: binding.JsRspackDiagnostic): void;
// @internal
__internal__setAssetSource(filename: string, source: Source): void;
// (undocumented)
get __internal__shutdown(): boolean;
set __internal__shutdown(shutdown: boolean);
// @internal
__internal_getInner(): binding.JsCompilation;
// (undocumented)
Expand Down Expand Up @@ -10022,7 +10025,7 @@ type StatOptions = {
export class Stats {
constructor(compilation: Compilation);
// (undocumented)
compilation: Compilation;
get compilation(): Compilation;
// (undocumented)
get endTime(): number | undefined;
// (undocumented)
Expand Down
10 changes: 10 additions & 0 deletions packages/rspack/src/Compilation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,7 @@ export type NormalizedStatsOptions = KnownNormalizedStatsOptions &

export class Compilation {
#inner: JsCompilation;
#shutdown: boolean;

hooks: Readonly<{
processAssets: liteTapable.AsyncSeriesHook<Assets>;
Expand Down Expand Up @@ -272,6 +273,7 @@ export class Compilation {

constructor(compiler: Compiler, inner: JsCompilation) {
this.#inner = inner;
this.#shutdown = false;
this.#customModules = {};

const processAssetsHook = new liteTapable.AsyncSeriesHook<Assets>([
Expand Down Expand Up @@ -1228,6 +1230,14 @@ BREAKING CHANGE: Asset processing hooks in Compilation has been merged into a si
return this.#inner;
}

get __internal__shutdown() {
return this.#shutdown;
}

set __internal__shutdown(shutdown) {
this.#shutdown = shutdown;
}

seal() {}
unseal() {}

Expand Down
7 changes: 6 additions & 1 deletion packages/rspack/src/Compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,12 @@ class Compiler {

this.hooks.shutdown.tap("rspack:cleanup", () => {
if (!this.running) {
this.#instance = undefined;
// Delayed rspack cleanup to the next tick.
// This supports calls to `fn rspack` to do something with `Stats` within the same tick.
process.nextTick(() => {
this.#instance = undefined;
this.#compilation && (this.#compilation.__internal__shutdown = true);
});
}
});
}
Expand Down
13 changes: 11 additions & 2 deletions packages/rspack/src/Stats.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,12 +23,12 @@ export type {

export class Stats {
#inner: binding.JsStats;
compilation: Compilation;
#compilation: Compilation;
#innerMap: WeakMap<Compilation, binding.JsStats>;

constructor(compilation: Compilation) {
this.#inner = compilation.__internal_getInner().getStats();
this.compilation = compilation;
this.#compilation = compilation;
this.#innerMap = new WeakMap([[this.compilation, this.#inner]]);
}

Expand All @@ -42,6 +42,15 @@ export class Stats {
return inner;
}

get compilation() {
if (this.#compilation.__internal__shutdown) {
throw new Error(
"Unable to access `Stats` after the compiler was shutdown"
);
}
return this.#compilation;
}

get hash() {
return this.compilation.hash;
}
Expand Down

2 comments on commit d249e62

@rspack-bot
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Benchmark detail: Open

Name Base (2024-11-28 9b70e3a) Current Change
10000_big_production-mode_disable-minimize + exec 42.8 s ± 992 ms 42.6 s ± 932 ms -0.63 %
10000_development-mode + exec 1.8 s ± 38 ms 1.79 s ± 52 ms -0.63 %
10000_development-mode_hmr + exec 648 ms ± 11 ms 649 ms ± 11 ms +0.27 %
10000_production-mode + exec 2.4 s ± 24 ms 2.39 s ± 25 ms -0.33 %
arco-pro_development-mode + exec 1.75 s ± 60 ms 1.75 s ± 87 ms -0.40 %
arco-pro_development-mode_hmr + exec 426 ms ± 2.4 ms 426 ms ± 1.9 ms -0.03 %
arco-pro_production-mode + exec 3.14 s ± 78 ms 3.1 s ± 38 ms -1.29 %
arco-pro_production-mode_generate-package-json-webpack-plugin + exec 3.14 s ± 85 ms 3.18 s ± 89 ms +1.04 %
threejs_development-mode_10x + exec 1.63 s ± 20 ms 1.62 s ± 18 ms -0.30 %
threejs_development-mode_10x_hmr + exec 812 ms ± 8.2 ms 805 ms ± 15 ms -0.87 %
threejs_production-mode_10x + exec 4.93 s ± 27 ms 4.92 s ± 25 ms -0.17 %
10000_big_production-mode_disable-minimize + rss memory 13461 MiB ± 268 MiB 13578 MiB ± 264 MiB +0.87 %
10000_development-mode + rss memory 803 MiB ± 57.1 MiB 787 MiB ± 45.3 MiB -2.06 %
10000_development-mode_hmr + rss memory 2077 MiB ± 208 MiB 2015 MiB ± 522 MiB -2.99 %
10000_production-mode + rss memory 661 MiB ± 25.9 MiB 678 MiB ± 42.4 MiB +2.50 %
arco-pro_development-mode + rss memory 725 MiB ± 34.1 MiB 738 MiB ± 46 MiB +1.81 %
arco-pro_development-mode_hmr + rss memory 929 MiB ± 99.5 MiB 933 MiB ± 126 MiB +0.39 %
arco-pro_production-mode + rss memory 848 MiB ± 72.3 MiB 870 MiB ± 48.1 MiB +2.61 %
arco-pro_production-mode_generate-package-json-webpack-plugin + rss memory 866 MiB ± 73.3 MiB 890 MiB ± 67.5 MiB +2.78 %
threejs_development-mode_10x + rss memory 838 MiB ± 47.5 MiB 837 MiB ± 37.5 MiB -0.17 %
threejs_development-mode_10x_hmr + rss memory 2157 MiB ± 476 MiB 2054 MiB ± 614 MiB -4.80 %
threejs_production-mode_10x + rss memory 1019 MiB ± 75.9 MiB 1052 MiB ± 71.3 MiB +3.29 %

@rspack-bot
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📝 Ran ecosystem CI: Open

suite result
modernjs ✅ success
_selftest ✅ success
rsdoctor ❌ failure
rspress ✅ success
rslib ❌ failure
rsbuild ✅ success
examples ✅ success
devserver ✅ success
nuxt ✅ success

Please sign in to comment.