facebook / memlab

A framework for finding JavaScript memory leaks and analyzing heap snapshots
https://facebook.github.io/memlab/
MIT License
4.37k stars 119 forks source link

Puppeteer browser.close() hangs on AWS Lambda #95

Closed doteric closed 4 months ago

doteric commented 1 year ago

Hello 👋

I have been having a problem running MemLab in an AWS Lambda (NodeJS v18) timing out no matter what time it is given and decided to investigate what's wrong. After a bit of investigation trying with and wihtout the warmup it looked like MemLab does all the needed work, but just fails to finish off and end, looked into it further and it seems to be an exact same problem that I had somewhere else with puppeteer. https://github.com/Sparticuz/chromium/issues/85#issuecomment-1549894734
The issue was that the browser can not close properly for some reason, but when all the pages are closed beforehand then it closes all fine. However this is not the case in this situation and I am still experiencing the browser.close() hanging even with the following code of MemLab:

async function closePuppeteer(
  browser: Browser,
  pages: Page[],
  options: AnyOptions = {},
): Promise<void> {
  if (config.isLocalPuppeteer && !options.warmup) {
    await Promise.all(pages.map(page => page.close()));
    await browser.disconnect();
  } else {
    const allPages = await browser.pages();
    for (const page of allPages) {
      await page.close();
    }
    await browser.close();
  }
}

Any other ideas what could be wrong?

To add to this I could not use the defined pages as the passed array is [page] (inside testInBrowser) and does not include any other pages (not expected, but possible) therefore it is safer to just look for all the pages again with await browser.pages()

Puppeteer chromium flags just FYI if they would matter by any chance (one part from MemLab, other part from @sparticuz/chromium.

memlabConfig.puppeteerConfig.args [
  '--no-sandbox',
  '--disable-notifications',
  '--use-fake-ui-for-media-stream',
  '--use-fake-device-for-media-stream',
  '--js-flags="--no-move-object-start"',
  '--enable-precise-memory-info',
  'browser-test',
  '--allow-pre-commit-input',
  '--disable-background-networking',
  '--disable-background-timer-throttling',
  '--disable-backgrounding-occluded-windows',
  '--disable-breakpad',
  '--disable-client-side-phishing-detection',
  '--disable-component-extensions-with-background-pages',
  '--disable-component-update',
  '--disable-default-apps',
  '--disable-dev-shm-usage',
  '--disable-extensions',
  '--disable-hang-monitor',
  '--disable-ipc-flooding-protection',
  '--disable-popup-blocking',
  '--disable-prompt-on-repost',
  '--disable-renderer-backgrounding',
  '--disable-sync',
  '--enable-automation',
  '--enable-blink-features=IdleDetection',
  '--export-tagged-pdf',
  '--force-color-profile=srgb',
  '--metrics-recording-only',
  '--no-first-run',
  '--password-store=basic',
  '--use-mock-keychain',
  '--disable-domain-reliability',
  '--disable-print-preview',
  '--disable-speech-api',
  '--disk-cache-size=33554432',
  '--mute-audio',
  '--no-default-browser-check',
  '--no-pings',
  '--single-process',
  '--disable-features=Translate,BackForwardCache,AcceptCHFrame,MediaRouter,OptimizationHints,AudioServiceOutOfProcess,IsolateOrigins,site-per-process',
  '--enable-features=NetworkServiceInProcess2,SharedArrayBuffer',
  '--hide-scrollbars',
  '--ignore-gpu-blocklist',
  '--in-process-gpu',
  '--window-size=1920,1080',
  '--use-gl=angle',
  '--use-angle=swiftshader',
  '--allow-running-insecure-content',
  '--disable-setuid-sandbox',
  '--disable-site-isolation-trials',
  '--disable-web-security',
  '--no-sandbox',
  '--no-zygote',
  "--headless='new'"
]
JacksonGL commented 1 year ago

"--headless='new'" - Not 100% sure if this is the problem, but this seems to be the new headless mode that puppeteer is testing. Could you try --headless=true and see if the browser can close without hanging (with your code that closes all the pages before closing the browser)?

doteric commented 1 year ago

Thanks @JacksonGL for the idea 💪
Sadly --headless=true + closing all pages before closing the browser does not seem to fix the issue with browser.close() hanging.
For now I will introduce a bypass to waiting for browser.close() to finish, but of course it's far from ideal so if you have any other ideas what could be wrong then I would highly appreciate it 🙇

doteric commented 4 months ago

Hello @JacksonGL

I assume you haven't looked into this problem?
Would you be okey for me to create a PR that contains an option to disable waiting for the browser to close? What do you think of that? Or maybe you have some other better approach on the back of your head?

Thank you

JacksonGL commented 4 months ago

@doteric Feel free to create a PR, I am happy to review it

doteric commented 4 months ago

@JacksonGL
MR created https://github.com/facebook/memlab/pull/120/files Please let me know if it's correct

JacksonGL commented 4 months ago

@doteric I left a comment on the PR.

JacksonGL commented 4 months ago

@doteric Please check and accept the CLA agreement so I can import and integrate your PR.

https://github.com/facebook/memlab/blob/main/CONTRIBUTING.md#contributor-license-agreement-cla

JacksonGL commented 4 months ago

Closing this as the PR is released in memlab@1.1.48 .