Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Temp files not cleaned up in WSL #15610

Open
2 tasks done
chrisrrowland opened this issue Nov 14, 2023 · 7 comments
Open
2 tasks done

Temp files not cleaned up in WSL #15610

chrisrrowland opened this issue Nov 14, 2023 · 7 comments

Comments

@chrisrrowland
Copy link

FAQ

URL

https://www.google.com

What happened?

In WSL I am running this code like

node LighthouseScanner.mjs https://www.google.com

After it completes there is a temp folder left behind that looks like a full windows path.

lighthouse ls -l
total 20
drwx------ 3 rowc rowc 4096 Nov 14 15:36 'C:\Users\chris\AppData\Local\lighthouse.42200815'

I don't see anything indicating an error.

import lighthouse from 'lighthouse';
import * as chromeLauncher from 'chrome-launcher';

(async () => {
  const chrome = await chromeLauncher.launch({
    chromeFlags: ['--headless', '--quiet'],
    chromePath: '/usr/bin/google-chrome-stable',
  });

  if (process.argv.length < 3) {
    console.error('Please provide a URL as a command line argument.');
    process.exit(1);
  }

  const url = process.argv[2];
  const options = {
    logLevel: 'info',
    output: ['html', 'json'],
    port: chrome.port,
    tmpdir: '/tmp/',
  };
  const runnerResult = await lighthouse(url, options);

  const combinedResult = {
    json: runnerResult.lhr,
    html: runnerResult.report,
  };

  // Output the combined result as a JSON object to stdout
  console.log(JSON.stringify(combinedResult, null, 2));

  await chrome.kill();
})();

What did you expect?

No temp files left behind.

What have you tried?

I have run it in directories not named lighthouse and the file name is still the same.

How were you running Lighthouse?

node

Lighthouse Version

11.2.0

Chrome Version

Google Chrome 118.0.5993.117

Node Version

v20.9.0

OS

WSL

Relevant log output

No response

@connorjclark
Copy link
Collaborator

This is due to chrome-launcher.

  1. https://github.com/GoogleChrome/chrome-launcher/blob/main/src/utils.ts has os-specific functions for makeTmpDir. Can we just use os.tmpdir? This is...overly complicated and there's no comments justifying doing this manually. If we use os.tmpdir it wouldn't matter as much that we do not clear them on early terminations of the process.

  2. Should we do [kill()](https://github.com/GoogleChrome/chrome-launcher/blob/main/src/chrome-launcher.ts#L399C3-L399C8) in a nodejs exit handler?

@chrisrrowland
Copy link
Author

This is due to chrome-launcher.

That's interesting, I opened an issue over there a couple of weeks ago, but I assumed the temp file names meant this particular bit was a lighthouse issue.

@connorjclark
Copy link
Collaborator

connorjclark commented Nov 15, 2023

yeah I'm sure through a series of copy/paste we just left that in when extracting chrome-launcher out of lighthouse forever ago.

Small curiosity: this folder only has "lighthouse" in the name for win32/unix, not when under WSL, if I'm reading util.ts correctly. Since you are running on WSL, but still get this "lighthouse" named temp folder, I suppose our getPlatform() / isWSL code isn't working as we expected. I've never used WSL myself, and I believe this was 100% an external contribution, so I'll install WSL and play with it a bit to see what's going on.

@paulirish do you know why chrome-launcher does so much work to create a temp dir in a specific place, rather than just use os.tmpdir? Like, for WSL it tried to grep a user name from PATH to create a folder in that user's appdata, which just seems kinda intense. What's the importance of making these folders in a specific place? seems any ol' tmpdir given by the OS would suffice.

I'll give it a shot on my windows machine.

@paulirish
Copy link
Member

@paulirish do you know why chrome-launcher does so much work to create a temp dir in a specific place,

no idea.

@chrisrrowland
Copy link
Author

I suppose our getPlatform() / isWSL code isn't working as we expected.

I'm pretty sure isWsl is correctly identifying my environment as WSL but I don't think the regex is working correctly. I had thrown out the idea of a parameter to specify a temp dir but os.tmpdir seems like it makes more sense.

Thanks for your attention on the matter!

@rudiedirkx
Copy link

I'm getting even weirder paths after running lighthouse. ls -lAh:

drwx------   3 rudie rudie 4.0K Jan 16 09:36 '\\wsl.localhost\Ubuntu\var\www\dip\undefined\Users\undefined\AppData\Local\lighthouse.98673163'/

I'm in /var/www/dip and that's where I ran node node_modules/lighthouse/cli/index.js.

What are the 2 undefineds? With a special unicode (?) character? And a Windows username?

I have lighthouse@11.4.0 and chrome-launcher@1.1.0. It does work well, I didn't even expect that. Well done WSL and Lighthouse 👍

@rudiedirkx
Copy link

If I change getPlatform() to just return process.platform and never 'wsl', everything works perfectly. Maybe this is a WSL 1 vs 2 thing? Maybe wsl specific code was necessary in WSL 1, but that's what 'breaks' WSL 2, which is 'real' linux.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment