diff --git a/server/src/repositories/process.repository.spec.ts b/server/src/repositories/process.repository.spec.ts index a3f44bd78b..3dbe909d22 100644 --- a/server/src/repositories/process.repository.spec.ts +++ b/server/src/repositories/process.repository.spec.ts @@ -81,5 +81,34 @@ describe(ProcessRepository.name, () => { await pipeline(Readable.from(data()), process); }); + + it('should kill the child process when the stream is destroyed', async () => { + const process = sut.spawnDuplexStream('bash', ['-c', 'sleep 60']); + const realProcess = (process as never as { _process: ChildProcessWithoutNullStreams })._process; + + expect(realProcess.exitCode).toBeNull(); + + const exited = new Promise((resolve) => realProcess.once('exit', () => resolve())); + process.destroy(); + await exited; + + expect(realProcess.killed).toBe(true); + }); + + it('should kill the child when pipeline tears the stream down after a failure', async () => { + const process = sut.spawnDuplexStream('yes'); + const realProcess = (process as never as { _process: ChildProcessWithoutNullStreams })._process; + const failingSink = new Writable({ + write(_chunk, _encoding, callback) { + callback(new Error('sink exploded')); + }, + }); + + const exited = new Promise((resolve) => realProcess.once('exit', () => resolve())); + await expect(pipeline(process, failingSink)).rejects.toThrow('sink exploded'); + await exited; + + expect(realProcess.killed).toBe(true); + }); }); }); diff --git a/server/src/repositories/process.repository.ts b/server/src/repositories/process.repository.ts index f5e761a965..c9a793a62d 100644 --- a/server/src/repositories/process.repository.ts +++ b/server/src/repositories/process.repository.ts @@ -42,6 +42,13 @@ export class ProcessRepository { process.stdin.end(callback); } }, + + destroy(error, callback) { + if (process.exitCode === null && process.signalCode === null) { + process.kill(); + } + callback(error); + }, }); // stdout -> duplex diff --git a/server/src/services/database-backup.service.spec.ts b/server/src/services/database-backup.service.spec.ts index ac808af8ae..242eac9e5c 100644 --- a/server/src/services/database-backup.service.spec.ts +++ b/server/src/services/database-backup.service.spec.ts @@ -1,6 +1,6 @@ import { BadRequestException } from '@nestjs/common'; import { DateTime } from 'luxon'; -import { PassThrough, Readable } from 'node:stream'; +import { Duplex, PassThrough, Readable } from 'node:stream'; import { StorageCore } from 'src/cores/storage.core'; import { defaults, SystemConfig } from 'src/dtos/config.dto'; import { ImmichWorker, JobStatus, StorageFolder } from 'src/enum'; @@ -252,6 +252,25 @@ describe(DatabaseBackupService.name, () => { await expect(sut.handleBackupDatabase()).rejects.toThrow('error'); }); + it('should destroy the spawned processes if the write stream fails', async () => { + const spawned: Duplex[] = []; + mocks.process.spawnDuplexStream.mockImplementation(() => { + const duplex = mockDuplex()('command', 0, 'data', ''); + spawned.push(duplex); + return duplex; + }); + mocks.storage.createWriteStream.mockImplementation(() => { + throw new Error('ENOENT: no such file or directory'); + }); + + await expect(sut.handleBackupDatabase()).rejects.toThrow('ENOENT'); + + expect(spawned).toHaveLength(2); + for (const stream of spawned) { + expect(stream.destroyed).toBe(true); + } + }); + it('should fail if rename fails', async () => { mocks.storage.rename.mockRejectedValue(new Error('error')); await expect(sut.handleBackupDatabase()).rejects.toThrow('error'); diff --git a/server/src/services/database-backup.service.ts b/server/src/services/database-backup.service.ts index d99d0433e5..f4365bb5f1 100644 --- a/server/src/services/database-backup.service.ts +++ b/server/src/services/database-backup.service.ts @@ -2,7 +2,7 @@ import { BadRequestException, Injectable, Optional } from '@nestjs/common'; import { debounce } from 'lodash'; import { DateTime } from 'luxon'; import path, { basename } from 'node:path'; -import { PassThrough, Readable, Writable } from 'node:stream'; +import { Duplex, PassThrough, Readable, Writable } from 'node:stream'; import { pipeline } from 'node:stream/promises'; import semver from 'semver'; import { serverVersion } from 'src/constants'; @@ -236,21 +236,26 @@ export class DatabaseBackupService { const backupFilePath = path.join(StorageCore.getBaseFolder(StorageFolder.Backups), filename); const temporaryFilePath = `${backupFilePath}.tmp`; + let pgdump: Duplex | undefined; + let gzip: Duplex | undefined; + try { - const pgdump = this.processRepository.spawnDuplexStream(bin, args, { + pgdump = this.processRepository.spawnDuplexStream(bin, args, { env: { PATH: process.env.PATH, PGPASSWORD: databasePassword, }, }); - const gzip = this.processRepository.spawnDuplexStream('gzip', ['--rsyncable']); + gzip = this.processRepository.spawnDuplexStream('gzip', ['--rsyncable']); const fileStream = this.storageRepository.createWriteStream(temporaryFilePath); await pipeline(pgdump, gzip, fileStream); await this.storageRepository.rename(temporaryFilePath, backupFilePath); } catch (error) { this.logger.error(`Database Backup Failure: ${error}`); + pgdump?.destroy(); + gzip?.destroy(); await this.storageRepository .unlink(temporaryFilePath)