refactor: remove PID requirement

We do not need the PID anymore since we fixed the sessionId so that
a new one is generated even after the ext host crashes and restarts.

The PID previously allowed us to differentiate between ide instances with
the same sessionId but different PIDs

Signed-off-by: nkomonen-amazon <[email protected]>

Author: nkomonen-amazon

packages/core/src/shared/crashMonitoring.ts

@@ -18,6 +18,7 @@ import { getLogger } from './logger/logger'
 import { crashMonitoringDirName } from './constants'
 import { throwOnUnstableFileSystem } from './filesystemUtilities'
 import { withRetries } from './utilities/functionUtils'
+import { truncateUuid } from './crypto'
 
 const className = 'CrashMonitoring'
 
@@ -307,17 +308,15 @@ class CrashChecker {
                         timestamp: ext.lastHeartbeat,
                     })
 
-                    devLogger?.debug(
-                        `crashMonitoring: CRASH: following has crashed: pid ${ext.extHostPid} + sessionId: ${ext.sessionId}`
-                    )
+                    devLogger?.debug(`crashMonitoring: CRASH: following has crashed: sessionId: ${ext.sessionId}`)
                 })
             }
 
             if (isDevMode) {
-                const before = knownExts.map((i) => i.extHostPid)
-                const after = runningExts.map((i) => i.extHostPid)
+                const before = knownExts.map((i) => truncateUuid(i.sessionId))
+                const after = runningExts.map((i) => truncateUuid(i.sessionId))
                 // Sanity check: ENSURE THAT AFTER === ACTUAL or this implies that our data is out of sync
-                const afterActual = (await state.getAllExts()).map((i) => i.extHostPid)
+                const afterActual = (await state.getAllExts()).map((i) => truncateUuid(i.sessionId))
                 devLogger?.debug(
                     `crashMonitoring: CHECKED: Result of cleaning up stopped instances\nBEFORE: ${JSON.stringify(before)}\nAFTER:  ${JSON.stringify(after)}\nACTUAL: ${JSON.stringify(afterActual)}`
                 )
@@ -386,7 +385,6 @@ class TimeLag {
  */
 type MementoStateDependencies = {
     memento: vscode.Memento
-    pid: number
     sessionId: string
     workDirPath: string
     isDevMode: boolean
@@ -401,7 +399,6 @@ function getDefaultDependencies(): MementoStateDependencies {
         workDirPath: path.join(globals.context.globalStorageUri.fsPath),
         memento: globals.globalState as vscode.Memento,
         isStateStale: () => isNewOsSession(),
-        pid: process.pid,
         sessionId: _getSessionId(),
         isDevMode: getIsDevMode(),
         devLogger: getIsDevMode() ? getLogger() : undefined,
@@ -433,7 +430,6 @@ export class FileSystemState {
     constructor(protected readonly deps: MementoStateDependencies) {
         this.stateDirPath = path.join(this.deps.workDirPath, crashMonitoringDirName)
 
-        this.deps.devLogger?.debug(`crashMonitoring: pid: ${this.deps.pid}`)
         this.deps.devLogger?.debug(`crashMonitoring: sessionId: ${this.deps.sessionId.slice(0, 8)}-...`)
         this.deps.devLogger?.debug(`crashMonitoring: dir: ${this.stateDirPath}`)
     }
@@ -538,7 +534,6 @@ export class FileSystemState {
     protected get ext(): ExtInstance {
         return {
             sessionId: this.deps.sessionId,
-            extHostPid: this.deps.pid,
             isDebug: isDebugInstance() ? true : undefined,
         }
     }
@@ -550,9 +545,9 @@ export class FileSystemState {
      * - The Extension Host PID used in addition to the session ID should be good enough to uniquely identiy.
      */
     protected createExtId(ext: ExtInstance): ExtInstanceId {
-        return `${ext.extHostPid}_${ext.sessionId}`
+        return ext.sessionId
     }
-    private readonly fileSuffix = 'running'
+    private readonly fileSuffix = 'v1'
     private makeStateFilePath(ext: ExtInstance | ExtInstanceId) {
         const extId = typeof ext === 'string' ? ext : this.createExtId(ext)
         return path.join(this.stateDirPath, extId + `.${this.fileSuffix}`)
@@ -654,7 +649,6 @@ type ExtInstanceId = string
 
 /** The static metadata of an instance of this extension */
 export type ExtInstance = {
-    extHostPid: number
     sessionId: string
     lastHeartbeat?: number
     /**

packages/core/src/shared/crypto.ts

@@ -42,3 +42,15 @@ export function isUuid(uuid: string): boolean {
     const uuidPattern = /^[0-9a-f]{8}-[0-9a-f]{4}-[1-5][0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/i
     return uuidPattern.test(uuid)
 }
+
+/**
+ * Eg: 'aaaabbbb-cccc-dddd-eeee-ffffhhhhiiii' -> 'aaaa...iiii'
+ */
+export function truncateUuid(uuid: string) {
+    if (uuid.length !== 36) {
+        throw new Error(`Cannot truncate uuid of value: "${uuid}"`)
+    }
+
+    const cleanedUUID = uuid.replace(/-/g, '')
+    return `${cleanedUUID.substring(0, 4)}...${cleanedUUID.substring(cleanedUUID.length - 4)}`
+}

packages/core/src/test/shared/crashMonitoring.test.ts

@@ -10,7 +10,7 @@ import { CrashMonitoring, ExtInstance, crashMonitoringStateFactory } from '../..
 import { isCI } from '../../shared/vscode/env'
 import { getLogger } from '../../shared/logger/logger'
 import { SinonSandbox, createSandbox } from 'sinon'
-import { fs } from '../../shared'
+import { fs, randomUUID } from '../../shared'
 import path from 'path'
 
 class TestCrashMonitoring extends CrashMonitoring {
@@ -55,13 +55,11 @@ export const crashMonitoringTest = async () => {
 
     async function makeTestExtension(id: number, opts?: { isStateStale: () => Promise<boolean> }) {
         const isStateStale = opts?.isStateStale ?? (() => Promise.resolve(false))
-        const sessionId = `sessionId-${id}`
-        const pid = Number(String(id).repeat(6))
+        const sessionId = randomUUID()
 
         const state = await crashMonitoringStateFactory({
             workDirPath: testFolder.path,
             isStateStale,
-            pid,
             sessionId: sessionId,
             now: () => globals.clock.Date.now(),
             memento: globals.globalState,
@@ -71,7 +69,6 @@ export const crashMonitoringTest = async () => {
         const ext = new TestCrashMonitoring(state, checkInterval, true, false, getLogger())
         spawnedExtensions.push(ext)
         const metadata = {
-            extHostPid: pid,
             sessionId,
             lastHeartbeat: globals.clock.Date.now(),
             isDebug: undefined,
@@ -276,8 +273,7 @@ export const crashMonitoringTest = async () => {
             const state = await crashMonitoringStateFactory({
                 workDirPath: testFolder.path,
                 isStateStale: () => Promise.resolve(false),
-                pid: 1111,
-                sessionId: 'sessionId_1111',
+                sessionId: randomUUID(),
                 now: () => globals.clock.Date.now(),
                 memento: globals.globalState,
                 isDevMode: true,

packages/core/src/test/shared/crypto.test.ts

@@ -4,7 +4,7 @@
  */
 
 import assert from 'assert'
-import { isUuid, randomUUID } from '../../shared/crypto'
+import { isUuid, randomUUID, truncateUuid } from '../../shared/crypto'
 
 describe('crypto', function () {
     describe('randomUUID()', function () {
@@ -44,3 +44,23 @@ describe('crypto', function () {
         assert.equal(isUuid('47fe01cf-f37a-4e7c-b971-d10fe5897763'.toUpperCase()), true)
     })
 })
+
+describe('truncateUUID', function () {
+    it('should return the first 4 and last 4 characters of a valid UUID', function () {
+        const fullUUID = 'aaaabbbb-cccc-dddd-eeee-ffffhhhhiiii'
+        const result = truncateUuid(fullUUID)
+        assert.strictEqual(result, 'aaaa...iiii')
+    })
+
+    it('should handle a different valid UUID correctly', function () {
+        const fullUUID = '12340000-0000-0000-0000-000000005678'
+        const result = truncateUuid(fullUUID)
+        assert.strictEqual(result, '1234...5678')
+    })
+
+    it('should throw an error if the input is not 36 characters long', function () {
+        assert.throws(() => {
+            truncateUuid('invalid-uuid')
+        }, /Cannot truncate uuid of value: "invalid-uuid"/)
+    })
+})