Merge pull request #235 from Sebastian-Webster/fix-process-already-killed

Fix process already killed on Windows

Author: Sebastian-Webster

.github/workflows/os-compatibility.yml

@@ -33,6 +33,7 @@ jobs:
       - name: Run tests
         env:
           VERSION_REQUIREMENT: ${{ matrix.version-requirement }}
+          MOVE_MYSQLMSN_TO: ${{ runner.os == 'Windows' && 'C:\\Users\\RUNNER~1\\mysqlmsn' || '/tmp/mysqlmsn' }}
         run: npm run os-compat:ci
 
       - name: Upload mysqlmsn directory (Windows)

ciSetup.js

@@ -4,7 +4,6 @@ process.env.useCIDBPath = true;
 
 const GitHubActionsTempFolder = process.platform === 'win32' ? 'C:\\Users\\RUNNER~1\\mysqlmsn' : '/tmp/mysqlmsn'
 
-process.env.mysqlmsn_internal_DO_NOT_USE_databaseDirectoryPath = normalize(GitHubActionsTempFolder + '/dbs')
 process.env.mysqlmsn_internal_DO_NOT_USE_binaryDirectoryPath = normalize(GitHubActionsTempFolder + '/binaries')
 
 process.env.mysqlmsn_internal_DO_NOT_USE_deleteDBAfterStopped = false;

dist/src/constants.d.ts

@@ -8,7 +8,6 @@ export declare const LOG_LEVELS: {
 };
 declare const internalOptions: {
     deleteDBAfterStopped: string;
-    databaseDirectoryPath: string;
     binaryDirectoryPath: string;
     cli: string;
 };

src/constants.ts

@@ -32,7 +32,6 @@ export const LOG_LEVELS = {
 const internalOptions = {
     deleteDBAfterStopped: 'true',
     //mysqlmsn = MySQL Memory Server Node.js
-    databaseDirectoryPath: normalizePath(`${tmpdir()}/mysqlmsn/dbs`),
     binaryDirectoryPath: `${tmpdir()}/mysqlmsn/binaries`,
     cli: 'false'
 }

src/libraries/Executor.ts

@@ -36,18 +36,30 @@ class Executor {
     }
 
     async #killProcess(process: ChildProcess): Promise<boolean> {
-        let killed = false;
+        // If the process has already been killed, return true
+        if (process.kill(0) === false) {
+            this.logger.warn('Called #killProcess to kill mysqld but it has already been killed.')
+            return true
+        }
+
         if (os.platform() === 'win32') {
             const {error, stderr} = await this.#executeFile('taskkill', ['/pid', String(process.pid), '/t', '/f'])
-            if (!error && !stderr) {
-                killed = true;
-            } else {
-                this.logger.error(error || stderr)
+            const message = error || stderr
+
+            if (!message) {
+                return true
+            }
+
+            if (message.toString().includes('There is no running instance of the task')) {
+                this.logger.warn('Called #killProcess and tried to kill mysqld process but taskkill could not because it is not running. Error received:', message)
+                return true
             }
-        } else {
-            killed = process.kill()
+
+            this.logger.error(message, '| Error toString:', message.toString())
+            return false
         }
-        return killed;
+
+        return process.kill('SIGKILL')
     }
 
     //Returns a path to the binary if it should be deleted
@@ -134,8 +146,9 @@ class Executor {
                 fs.unwatchFile(errorLogFile)
 
                 if (signal) {
-                    this.logger.log('Exiting because of aborted signal.')
-                    return
+                    this.logger.log('Exiting because the process received a signal:', signal)
+                } else {
+                    this.logger.log('Exiting with code:', code)
                 }
 
                 let errorLog: string;
@@ -169,7 +182,9 @@ class Executor {
 
                 try {
                     if (getInternalEnvVariable('deleteDBAfterStopped') === 'true') {
+                        this.logger.log('Deleting database path as deleteDBAfterStopped is true...')
                         await fsPromises.rm(dbPath, {recursive: true, force: true, maxRetries: 50, retryDelay: 100})
+                        this.logger.log('Database deletion was successful.')
                     }
                 } catch (e) {
                     this.logger.error('An error occurred while deleting database directory at path:', dbPath, '| The error was:', e)  
@@ -527,7 +542,7 @@ class Executor {
 
         let retries = 0;
 
-        this.databasePath = normalizePath(`${getInternalEnvVariable('databaseDirectoryPath')}/${randomUUID().replaceAll("-", '')}`)
+        this.databasePath = normalizePath(`${os.tmpdir()}/mysqlmsn/dbs/${randomUUID().replaceAll("-", '')}`)
 
         const datadir = normalizePath(`${this.databasePath}/data`)
 

tests/versions.test.ts

@@ -1,10 +1,13 @@
-import {expect, test, jest} from '@jest/globals'
+import {expect, test, jest, afterAll} from '@jest/globals'
 import { createDB } from '../src/index'
 import sql from 'mysql2/promise'
 import { coerce, satisfies } from 'semver';
 import { ServerOptions } from '../types';
 import getBinaryURL from '../src/libraries/Version';
 import { DOWNLOADABLE_MYSQL_VERSIONS } from '../src/constants';
+import fs from 'fs'
+import fsPromises from 'fs/promises'
+import os from 'os'
 
 const usernames = ['root', 'dbuser']
 
@@ -59,4 +62,12 @@ for (const version of DOWNLOADABLE_MYSQL_VERSIONS.filter(v => satisfies(v, versi
 //binary, we need this test here just in case all the MySQL binaries are skipped
 test('dummy test', () => {
     expect(1 + 1).toBe(2)
+})
+
+afterAll(async () => {
+    const originalPath = `${os.tmpdir()}/mysqlmsn`
+    if (process.env.MOVE_MYSQLMSN_TO && fs.existsSync(originalPath) && originalPath !== process.env.MOVE_MYSQLMSN_TO) {
+        await fsPromises.cp(originalPath, process.env.MOVE_MYSQLMSN_TO, {recursive: true, force: true, filter: source => !source.includes('.sock')})
+        await fsPromises.rm(originalPath, {force: true, recursive: true, maxRetries: 50, retryDelay: 100})
+    }
 })