fix: preserve orphaned process electron

cli/CHANGELOG.md

@@ -5,6 +5,7 @@ _Released 5/21/2024 (PENDING)_
 
 **Bugfixes:**
 
+- Fixed an issue where orphaned Electron processes were inadvertently terminating the browser's CRI client. Fixes [#28397](https://github.com/cypress-io/cypress/issues/28397). Fixed in [#29515](https://github.com/cypress-io/cypress/pull/29515).
 - Fixed an issue where Cypress was unable to search in the Specs list for files or folders containing numbers. Fixes [#29034](https://github.com/cypress-io/cypress/issues/29034).
 
 **Dependency Updates:**

packages/server/lib/browsers/electron.ts

@@ -54,6 +54,7 @@ const _getAutomation = async function (win, options: BrowserLaunchOpts, parent)
   const port = getRemoteDebuggingPort()
 
   if (!browserCriClient) {
+    debug(`browser CRI is not set. Creating...`)
     browserCriClient = await BrowserCriClient.create({
       hosts: ['127.0.0.1'],
       port,
@@ -68,6 +69,7 @@ const _getAutomation = async function (win, options: BrowserLaunchOpts, parent)
   const pageCriClient = await browserCriClient.attachToTargetUrl('about:blank')
 
   const sendClose = async () => {
+    debug(`sendClose called, browserCriClient is set? ${!!browserCriClient}`)
     if (browserCriClient) {
       const gracefulShutdown = true
 
@@ -443,10 +445,15 @@ export = {
    * Clear instance state for the electron instance, this is normally called on kill or on exit, for electron there isn't any state to clear.
    */
   clearInstanceState (options: GracefulShutdownOptions = {}) {
-    debug('closing remote interface client', { options })
-    // Do nothing on failure here since we're shutting down anyway
-    browserCriClient?.close(options.gracefulShutdown).catch(() => {})
-    browserCriClient = null
+    // in the case of orphaned browser clients, we should preserve the CRI client as it is connected
+    // to the same host regardless of window
+    debug('clearInstanceState called with options', { options })
+    if (!options.shouldPreserveCriClient) {
+      debug('closing remote interface client')
+      // Do nothing on failure here since we're shutting down anyway
+      browserCriClient?.close(options.gracefulShutdown).catch(() => {})
+      browserCriClient = null
+    }
   },
 
   connectToNewSpec (browser: Browser, options: ElectronOpts, automation: Automation) {
@@ -534,7 +541,7 @@ export = {
       allPids: [mainPid],
       browserWindow: win,
       kill (this: BrowserInstance) {
-        clearInstanceState({ gracefulShutdown: true })
+        clearInstanceState({ gracefulShutdown: true, shouldPreserveCriClient: this.isOrphanedBrowserProcess })
 
         if (this.isProcessExit) {
           // if the process is exiting, all BrowserWindows will be destroyed anyways

packages/server/lib/browsers/index.ts

@@ -24,12 +24,14 @@ interface KillOptions {
   isProcessExit?: boolean
   nullOut?: boolean
   unbind?: boolean
+  isOrphanedBrowserProcess?: boolean
 }
 
 const kill = (options: KillOptions = {}) => {
   options = _.defaults({}, options, {
     instance,
     isProcessExit: false,
+    isOrphanedBrowserProcess: false,
     unbind: true,
     nullOut: true,
   })
@@ -60,7 +62,7 @@ const kill = (options: KillOptions = {}) => {
     debug('killing browser process')
 
     instanceToKill.isProcessExit = options.isProcessExit
-
+    instanceToKill.isOrphanedBrowserProcess = options.isOrphanedBrowserProcess
     instanceToKill.kill()
   })
 }
@@ -180,7 +182,7 @@ export = {
 
     const _instance = await browserLauncher.open(browser, options.url, options, automation, ctx.coreData.servers.cdpSocketServer)
 
-    debug('browser opened')
+    debug(`browser opened for launch ${thisLaunchAttempt}`)
 
     // in most cases, we'll kill any running browser instance before launching
     // a new one when we call `await kill()` early in this function.
@@ -204,8 +206,11 @@ export = {
     // this browser, it means it has been orphaned and should be terminated.
     //
     // https://github.com/cypress-io/cypress/issues/24377
-    if (thisLaunchAttempt !== launchAttempt) {
-      await kill({ instance: _instance, nullOut: false })
+    const isOrphanedBrowserProcess = thisLaunchAttempt !== launchAttempt
+
+    if (isOrphanedBrowserProcess) {
+      debug(`killing process because launch attempt: ${thisLaunchAttempt} does not match current launch attempt: ${launchAttempt}`)
+      await kill({ instance: _instance, isOrphanedBrowserProcess, nullOut: false })
 
       return null
     }

packages/server/lib/browsers/types.ts

@@ -27,6 +27,7 @@ export type BrowserInstance = EventEmitter & {
    * TODO: remove need for this
    */
   isProcessExit?: boolean
+  isOrphanedBrowserProcess?: boolean
 }
 
 export type BrowserLauncher = {
@@ -52,4 +53,5 @@ export type BrowserLauncher = {
 
 export type GracefulShutdownOptions = {
   gracefulShutdown?: boolean
+  shouldPreserveCriClient?: boolean
 }

packages/server/test/unit/browsers/browsers_spec.js

@@ -209,6 +209,7 @@ describe('lib/browsers/index', () => {
       browsers._setInstance(null)
 
       expect(browserInstance1.kill).to.be.calledOnce
+      expect(browserInstance1.isOrphanedBrowserProcess).to.be.true
       expect(currentInstance).to.equal(browserInstance2)
     })
 
@@ -262,6 +263,7 @@ describe('lib/browsers/index', () => {
       browsers._setInstance(null)
 
       expect(browserInstance1.kill).to.be.calledOnce
+      expect(browserInstance1.isOrphanedBrowserProcess).to.be.true
       expect(currentInstance).to.equal(browserInstance2)
     })
   })

packages/server/test/unit/browsers/electron_spec.js

@@ -262,6 +262,33 @@ describe('lib/browsers/electron', () => {
     })
   })
 
+  context('.kill', () => {
+    beforeEach(async function () {
+      await electron._getAutomation({}, { onError: () => {} }, {})
+
+      await this.stubForOpen()
+
+      sinon.stub(electron, '_getBrowserCriClient').returns(this.browserCriClient)
+    })
+
+    it('does not terminate the browserCriClient if the instance is an orphaned process', async function () {
+      const instance = await electron.open('electron', this.url, this.options, this.automation)
+
+      instance.isOrphanedBrowserProcess = true
+      instance.kill()
+
+      expect(this.browserCriClient.close).not.to.be.called
+    })
+
+    it('terminates the browserCriClient otherwise', async function () {
+      const instance = await electron.open('electron', this.url, this.options, this.automation)
+
+      instance.kill()
+
+      expect(this.browserCriClient.close).to.be.called
+    })
+  })
+
   context('._launch', () => {
     beforeEach(() => {
       sinon.stub(menu, 'set')