fix(migrate): don't kill the SE process on Windows immediately

Windows doesn't have signals, so calling `this.child?.kill()`
immediately terminates the child process without running the
graceful shutdown code. We should trigger the graceful shutdown
by closing stdin instead.

Depends on <prisma/prisma-engines#5486>.

Closes: https://linear.app/prisma-company/issue/ORM-1093/schema-engine-termination-on-windows

Author: aqrln

packages/migrate/src/Migrate.ts

@@ -62,8 +62,8 @@ export class Migrate {
     return new Migrate({ engine, schemaContext, ...rest })
   }
 
-  public stop(): void {
-    this.engine.stop()
+  public async stop(): Promise<void> {
+    await this.engine.stop()
   }
 
   public getPrismaSchema(): MigrateTypes.SchemasContainer {

packages/migrate/src/SchemaEngine.ts

@@ -125,5 +125,5 @@ export interface SchemaEngine {
   /**
    * Stop the engine.
    */
-  stop(): void
+  stop(): Promise<void>
 }

packages/migrate/src/SchemaEngineCLI.ts

@@ -201,7 +201,7 @@ export class SchemaEngineCLI implements SchemaEngine {
     } finally {
       // stop the engine after either a successful or failed introspection, to emulate how the
       // introspection engine used to work.
-      this.stop()
+      await this.stop()
     }
   }
 
@@ -263,12 +263,37 @@ export class SchemaEngineCLI implements SchemaEngine {
     return this.runCommand(this.getRPCPayload('introspectSql', args))
   }
 
-  public stop(): void {
-    if (this.child) {
-      this.child.stdin?.end()
-      this.child.kill()
-      this.isRunning = false
+  public async stop(): Promise<void> {
+    if (!this.child) {
+      return
     }
+
+    this.isRunning = false
+
+    // Close stdin to tell the schema engine to stop the RPC server and exit.
+    this.child.stdin?.end()
+
+    // Terminate the child process after a timeout if it's still running. On
+    // Unix, this will send a SIGTERM signal to the process, which the schema
+    // engine will handle and initiate a graceful shutdown. Since it has its
+    // own timeout mechanism, we shouldn't wait before sending the signal,
+    // otherwise these timeouts will add up. On Windows, there are no signals
+    // and the kill method will terminate the process immediately, similar to
+    // SIGKILL on Unix, so we should wait to give the schema engine a chance
+    // to gracefully shut down in response to EOF in stdin.
+    const timer = setTimeout(
+      () => {
+        this.child?.kill()
+      },
+      process.platform === 'win32' ? 4000 : 0,
+    ).unref()
+
+    return new Promise((resolve) => {
+      this.child!.on('exit', () => {
+        clearTimeout(timer)
+        resolve()
+      })
+    })
   }
 
   private rejectAll(err: any): void {

packages/migrate/src/SchemaEngineWasm.ts

@@ -300,9 +300,10 @@ export class SchemaEngineWasm implements SchemaEngine {
   /**
    * Stop the engine.
    */
-  public stop(): void {
+  public stop(): Promise<void> {
     this.isRunning = false
     this.engine.free()
+    return Promise.resolve()
   }
 }
 

packages/migrate/src/__tests__/DbPull/postgresql-views.test.ts

@@ -83,7 +83,7 @@ describeMatrix(postgresOnly, 'postgresql-views', () => {
         })
 
         expect(introspectionResult.views).toEqual(null)
-        engine.stop()
+        await engine.stop()
       })
     })
 
@@ -105,7 +105,7 @@ describeMatrix(postgresOnly, 'postgresql-views', () => {
         })
 
         expect(introspectionResult.views).toEqual([])
-        engine.stop()
+        await engine.stop()
 
         const listWithoutViews = await ctx.fs.listAsync('views')
         expect(listWithoutViews).toEqual(undefined)
@@ -132,7 +132,7 @@ describeMatrix(postgresOnly, 'postgresql-views', () => {
         })
 
         expect(introspectionResult.views).toEqual([])
-        engine.stop()
+        await engine.stop()
 
         const listWithoutViews = await ctx.fs.listAsync('views')
         expect(listWithoutViews).toEqual(undefined)
@@ -159,7 +159,7 @@ describeMatrix(postgresOnly, 'postgresql-views', () => {
         })
 
         expect(introspectionResult.views).toEqual([])
-        engine.stop()
+        await engine.stop()
 
         const listWithoutViews = await ctx.fs.listAsync('views')
         expect(listWithoutViews).toEqual(['README.md'])

packages/migrate/src/__tests__/rpc.test.ts

@@ -30,7 +30,7 @@ describe('applyMigrations', () => {
         "appliedMigrationNames": [],
       }
     `)
-    migrate.stop()
+    await migrate.stop()
   })
 
   it('should fail on existing brownfield db', async () => {
@@ -49,7 +49,7 @@ describe('applyMigrations', () => {
       The database schema is not empty. Read more about how to baseline an existing production database: https://pris.ly/d/migrate-baseline
       "
     `)
-    migrate.stop()
+    await migrate.stop()
   })
 })
 
@@ -69,7 +69,7 @@ describe('createDatabase', () => {
         "databaseName": "dev.db",
       }
     `)
-    migrate.stop()
+    await migrate.stop()
   })
 
   it('should succeed - Schema - postgresql', async () => {
@@ -89,7 +89,7 @@ describe('createDatabase', () => {
       Database \`tests\` already exists on the database server
       "
     `)
-    migrate.stop()
+    await migrate.stop()
   })
 })
 
@@ -110,7 +110,7 @@ describe('createMigration', () => {
         "generatedMigrationName": "20201231000000_my_migration",
       }
     `)
-    migrate.stop()
+    await migrate.stop()
   })
 
   it('draft should succeed - existing-db-1-migration', async () => {
@@ -129,7 +129,7 @@ describe('createMigration', () => {
         "generatedMigrationName": "20201231000000_draft_123",
       }
     `)
-    migrate.stop()
+    await migrate.stop()
   })
 })
 
@@ -148,7 +148,7 @@ describe('dbExecute', () => {
     })
 
     await expect(result).resolves.toMatchInlineSnapshot(`null`)
-    migrate.stop()
+    await migrate.stop()
   })
 })
 
@@ -170,7 +170,7 @@ describe('devDiagnostic', () => {
       }
     `)
 
-    migrate.stop()
+    await migrate.stop()
   })
 
   it('reset because drift', async () => {
@@ -203,7 +203,7 @@ describe('devDiagnostic', () => {
       }
     `)
 
-    migrate.stop()
+    await migrate.stop()
   })
 })
 
@@ -227,7 +227,7 @@ describe('diagnoseMigrationHistory', () => {
         "history": null,
       }
     `)
-    migrate.stop()
+    await migrate.stop()
   })
 
   it(' optInToShadowDatabase false should succeed - existing-db-1-migration', async () => {
@@ -249,7 +249,7 @@ describe('diagnoseMigrationHistory', () => {
         "history": null,
       }
     `)
-    migrate.stop()
+    await migrate.stop()
   })
 })
 
@@ -266,7 +266,7 @@ describe('ensureConnectionValidity', () => {
     })
 
     await expect(result).resolves.toMatchInlineSnapshot(`{}`)
-    migrate.stop()
+    await migrate.stop()
   })
 
   it('should succeed when database exists - PostgreSQL', async () => {
@@ -282,7 +282,7 @@ describe('ensureConnectionValidity', () => {
     })
 
     await expect(result).resolves.toMatchInlineSnapshot(`{}`)
-    migrate.stop()
+    await migrate.stop()
   })
 
   it('should fail when database does not exist - SQLite', async () => {
@@ -301,7 +301,7 @@ describe('ensureConnectionValidity', () => {
       Database \`dev.db\` does not exist
       "
     `)
-    migrate.stop()
+    await migrate.stop()
   })
 
   it('should fail when server does not exist - PostgreSQL', async () => {
@@ -322,7 +322,7 @@ describe('ensureConnectionValidity', () => {
       Please make sure your database server is running at \`server-does-not-exist:5432\`.
       "
     `)
-    migrate.stop()
+    await migrate.stop()
     // It was flaky on CI (but rare)
     // higher timeout might help
   }, 10_000)
@@ -348,7 +348,7 @@ describe('evaluateDataLoss', () => {
         "warnings": [],
       }
     `)
-    migrate.stop()
+    await migrate.stop()
   })
 
   // migration is already applied so should be empty
@@ -370,7 +370,7 @@ describe('evaluateDataLoss', () => {
         "warnings": [],
       }
     `)
-    migrate.stop()
+    await migrate.stop()
   })
 })
 
@@ -382,7 +382,7 @@ describe('getDatabaseVersion - PostgreSQL', () => {
     const migrate = await Migrate.setup({ migrationsDirPath, schemaContext })
     const result = migrate.engine.getDatabaseVersion()
     await expect(result).resolves.toContain('PostgreSQL')
-    migrate.stop()
+    await migrate.stop()
   })
 
   it('[SchemaPath] should succeed', async () => {
@@ -396,7 +396,7 @@ describe('getDatabaseVersion - PostgreSQL', () => {
       },
     })
     await expect(result).resolves.toContain('PostgreSQL')
-    migrate.stop()
+    await migrate.stop()
   })
 
   it('[ConnectionString] should succeed', async () => {
@@ -408,7 +408,7 @@ describe('getDatabaseVersion - PostgreSQL', () => {
       },
     })
     await expect(result).resolves.toContain('PostgreSQL')
-    migrate.stop()
+    await migrate.stop()
   })
 })
 
@@ -432,7 +432,7 @@ describe('markMigrationRolledBack', () => {
       "
     `)
 
-    migrate.stop()
+    await migrate.stop()
   })
 
   it('existing-db-1-migration', async () => {
@@ -493,7 +493,7 @@ describe('markMigrationRolledBack', () => {
       "
     `)
 
-    migrate.stop()
+    await migrate.stop()
   })
 })
 
@@ -523,7 +523,7 @@ describe('markMigrationApplied', () => {
 
     await expect(resultMarkApplied).resolves.toMatchInlineSnapshot(`{}`)
 
-    migrate.stop()
+    await migrate.stop()
   })
 })
 
@@ -547,7 +547,7 @@ describe('schemaPush', () => {
         "warnings": [],
       }
     `)
-    migrate.stop()
+    await migrate.stop()
   })
 
   it('should succeed without warning', async () => {
@@ -567,7 +567,7 @@ describe('schemaPush', () => {
         "warnings": [],
       }
     `)
-    migrate.stop()
+    await migrate.stop()
   })
 
   it('should return executedSteps 0 with warning if dataloss detected', async () => {
@@ -589,7 +589,7 @@ describe('schemaPush', () => {
         ],
       }
     `)
-    migrate.stop()
+    await migrate.stop()
   })
 
   it('force should accept dataloss', async () => {
@@ -611,7 +611,7 @@ describe('schemaPush', () => {
         ],
       }
     `)
-    migrate.stop()
+    await migrate.stop()
   })
 })
 

packages/migrate/src/commands/DbExecute.ts

@@ -207,7 +207,7 @@ See \`${green(getCommandWithExecutor('prisma db execute -h'))}\``,
         datasourceType,
       })
     } finally {
-      migrate.stop()
+      await migrate.stop()
     }
 
     return `Script executed successfully.`