webpack-contrib / sass-loader

Compiles Sass to CSS
MIT License
3.91k stars 431 forks source link

Extra `sass-embedded` compilers are not disposed #1244

Closed hikiko4ern closed 3 weeks ago

hikiko4ern commented 3 weeks ago

Bug report

When using the sass-embedded with the modern-compiler API, sass-loader may create multiple compilers (= make multiple initAsyncCompiler calls). Before the created compiler is saved to sassModernCompilers, there is a check to handle race conditions: https://github.com/webpack-contrib/sass-loader/blob/c75c6061266a6df6a7c736de407fcbf05ef5b2be/src/utils.js#L764-L766

But in the case when sassModernCompilers.has(webpackCompiler) === true, the newly created compiler is not disposed (i.e. compiler.dispose() is not called), causing sass-embedded to never shut down the dart sass.snapshot --embedded process it started, causing the Node.js process to never exit due to alive child_processes.

Actual Behavior

Node.js process does not exit due to alive child processes

Expected Behavior

If sassModernCompilers already has a compiler, should compiler.dispose() be called for the newly created compiler? Something like

 if (!sassModernCompilers.has(webpackCompiler)) {
   sassModernCompilers.set(webpackCompiler, compiler);
   webpackCompiler.hooks.shutdown.tap("sass-loader", () => {
     compiler.dispose();
   });
-}
+} else {
+  compiler.dispose();
+}

How Do We Reproduce?

Unfortunately, even on our closed project with a large number of files, the bug does not reproduce consistently. But it can be easily reproduced by adding a small delay: https://github.com/hikiko4ern/sass-loader-compiler/blob/master/patches/sass-loader.patch

  1. clone https://github.com/hikiko4ern/sass-loader-compiler
  2. run pnpm i
  3. run pnpm webpack

It should hang after

webpack 5.95.0 compiled successfully in 233 ms

Please paste the results of npx webpack-cli info here, and mention other relevant information

System:
  OS: Linux 5.15 Debian GNU/Linux trixie/sid
  CPU: (32) x64 AMD Ryzen 9 7950X3D 16-Core Processor
  Memory: 53.94 GB / 64.76 GB
Binaries:
  Node: 22.11.0 - /run/user/1000/fnm_multishells/1007232_1730374215459/bin/node
  npm: 10.9.0 - /run/user/1000/fnm_multishells/1007232_1730374215459/bin/npm
Packages:
  sass-loader: ^16.0.2 => 16.0.2
  webpack: ^5.95.0 => 5.95.0
  webpack-cli: ^5.1.4 => 5.1.4
alexander-akait commented 3 weeks ago

@hikiko4ern pr welcome

KidkArolis commented 3 weeks ago

After upgrading sass-loader to 16.0.3 I still experience the issue where the rspack build process does not exit, something is still holding it up.

alexander-akait commented 3 weeks ago

Maybe rspack doesn't support https://github.com/webpack-contrib/sass-loader/blob/master/src/utils.js#L768

powah commented 1 week ago

Yes, I am experiencing the same issue - after successful build the process never exits. If I change to api: "modern" all is good