[Real Browser] close the browser after getBrowser()

[Ali-Razmjoo]

Tick the checkbox if you understand [x]:

• I have read and understand the pull request rules.

Description

This fixes High CPU Usage after enabling the Web Driver (#3788).

This PR focuses on improving the cleanup process after each monitoring check in the server/monitor-types/real-browser-monitor-type.js file. The main changes include the introduction of a new cleanupBrowser function, which is responsible for terminating all browser processes and clearing the cache, and the replacement of the context.close() method with the new cleanupBrowser function in the RealBrowserMonitorType class.

Here are the key changes:

• server/monitor-types/real-browser-monitor-type.js: Added a new cleanupBrowser function that terminates all browser processes and clears the cache after each monitoring check. This function loops through all contexts of the browser, clears cookies and permissions, and then closes the context.
• server/monitor-types/real-browser-monitor-type.js: Replaced the context.close() method with the new cleanupBrowser function in the RealBrowserMonitorType class. This ensures that the cleanup process is executed after each monitoring check.

Type of change

Please delete any options that are not relevant.

• Bug fix (non-breaking change which fixes an issue)

Checklist

• My code follows the style guidelines of this project
• I ran ESLint and other linters for modified files
• I have performed a self-review of my own code and tested it
• I have commented my code, particularly in hard-to-understand areas
  (including JSDoc for methods)
• My changes generate no new warnings
• My code needed automated testing. I have added them (this is optional task)

Screenshots (if any)

• Not applicable

[chakflying]

This probably works, but it may be better for getBrowser to return an existing instance, and just open/close tabs instead. That way we save even more resources by not having to go to the kernel for a new process.

[louislam]

Haven't look into this issue yet, but by my design, one browser belongs to one monitor. If the browser closed in here, the next beat should throw an exception unexpectedly.

The getBrowser is actually returning the same instance.

[Ali-Razmjoo]

@louislam I have reviewed the code every time check function is called, inside that you get new browser by const browser = await getBrowser();. Inside of that code if the browser is closed it opens a new one; but not sure if opening and closing are efficient, but better than high CPU usage...

async function getBrowser() {
    if (!browser) {
        let executablePath = await Settings.get("chromeExecutable");

        executablePath = await prepareChromeExecutable(executablePath);

        browser = await chromium.launch({
            //headless: false,
            executablePath,
        });
    }
    return browser;
}

[louislam]

Just tested. As I expected before. The monitor is crashed in the second tick. Browser cannot be closed in the middle of the tick.

Still not clear what causes #3788. I guess it is Linux only bug, as I cannot reproduce #3788 on Windows.

[Ali-Razmjoo]

@louislam I updated the code. Can you give me some thoughts/insights on what needs to be done to get rid of this issue?

[Ali-Razmjoo]

forgot the screenshots...

I added a test, and let it run for a while to ensure it's working...

then I started watching the processes

seems to be working, but unsure if I missed anything.

---

update: Now I am wondering if it closes context from other monitors? in that case we can maybe close a singular context...

[louislam]

For the current design, Uptime Kuma creates one browser instance for multiple monitors.
So I am afraid that your new fix is still not working, as closing all contexts will probably affect other monitors.

As I said before, the current code is working fine on Windows.
Uncommenting headless: false, can see behind the screens.

However, I don't have a desktop environment right now on Linux, I can't debug it with headless: false on Linux

[Ali-Razmjoo]

For the current design, Uptime Kuma creates one browser instance for multiple monitors.
So I am afraid that your new fix is still not working, as closing all contexts will probably affect other monitors.

that's what I suspected... I will send a fix now.

As I said before, the current code is working fine on Windows.
Uncommenting headless: false, can see behind the screens.

As I posted in
#3788 there are many chromium processes/tabs left opened after check() is called, and needs to be closed to prevent the DoS.