Debt: avoid promise.cancel in zip.ts

src/vs/base/node/extfs.ts

@@ -15,6 +15,7 @@ import * as uuid from 'vs/base/common/uuid';
 import { TPromise } from 'vs/base/common/winjs.base';
 import { encode, encodeStream } from 'vs/base/node/encoding';
 import * as flow from 'vs/base/node/flow';
+import { CancellationToken } from 'vs/base/common/cancellation';
 
 const loop = flow.loop;
 
@@ -129,7 +130,7 @@ function doCopyFile(source: string, target: string, mode: number, callback: (err
 	reader.pipe(writer);
 }
 
-export function mkdirp(path: string, mode?: number): TPromise<boolean> {
+export function mkdirp(path: string, mode?: number, token?: CancellationToken): TPromise<boolean> {
 	const mkdir = () => {
 		return nfcall(fs.mkdir, path, mode).then(null, (mkdirErr: NodeJS.ErrnoException) => {
 
@@ -160,6 +161,11 @@ export function mkdirp(path: string, mode?: number): TPromise<boolean> {
 	// recursively mkdir
 	return mkdir().then(null, (err: NodeJS.ErrnoException) => {
 
+		// Respect cancellation
+		if (token && token.isCancellationRequested) {
+			return TPromise.as(false);
+		}
+
 		// ENOENT: a parent folder does not exist yet, continue
 		// to create the parent folder and then try again.
 		if (err.code === 'ENOENT') {

src/vs/base/node/zip.ts

@@ -7,11 +7,13 @@ import * as nls from 'vs/nls';
 import * as path from 'path';
 import { createWriteStream, WriteStream } from 'fs';
 import { Readable } from 'stream';
-import { nfcall, ninvoke, SimpleThrottler } from 'vs/base/common/async';
+import { nfcall, ninvoke, SimpleThrottler, createCancelablePromise, CancelablePromise } from 'vs/base/common/async';
 import { mkdirp, rimraf } from 'vs/base/node/pfs';
 import { TPromise } from 'vs/base/common/winjs.base';
 import { open as _openZip, Entry, ZipFile } from 'yauzl';
 import { ILogService } from 'vs/platform/log/common/log';
+import { CancellationToken } from 'vs/base/common/cancellation';
+import { once } from 'vs/base/common/event';
 
 export interface IExtractOptions {
 	overwrite?: boolean;
@@ -69,7 +71,7 @@ function toExtractError(err: Error): ExtractError {
 	return new ExtractError(type, err);
 }
 
-function extractEntry(stream: Readable, fileName: string, mode: number, targetPath: string, options: IOptions): TPromise<void> {
+function extractEntry(stream: Readable, fileName: string, mode: number, targetPath: string, options: IOptions, token: CancellationToken): TPromise<void> {
 	const dirName = path.dirname(fileName);
 	const targetDirName = path.join(targetPath, dirName);
 	if (targetDirName.indexOf(targetPath) !== 0) {
@@ -78,72 +80,88 @@ function extractEntry(stream: Readable, fileName: string, mode: number, targetPa
 	const targetFileName = path.join(targetPath, fileName);
 
 	let istream: WriteStream;
-	return mkdirp(targetDirName).then(() => new TPromise((c, e) => {
+
+	once(token.onCancellationRequested)(() => {
+		if (istream) {
+			istream.close();
+		}
+	});
+
+	return mkdirp(targetDirName, void 0, token).then(() => new TPromise((c, e) => {
+		if (token.isCancellationRequested) {
+			return;
+		}
+
 		istream = createWriteStream(targetFileName, { mode });
 		istream.once('close', () => c(null));
 		istream.once('error', e);
 		stream.once('error', e);
 		stream.pipe(istream);
-	}, () => {
-		if (istream) {
-			istream.close();
-		}
 	}));
 }
 
-function extractZip(zipfile: ZipFile, targetPath: string, options: IOptions, logService: ILogService): TPromise<void> {
+function extractZip(zipfile: ZipFile, targetPath: string, options: IOptions, logService: ILogService): CancelablePromise<void> {
 	let isCanceled = false;
-	let last = TPromise.wrap<any>(null);
+	let last = createCancelablePromise(() => Promise.resolve(null));
 	let extractedEntriesCount = 0;
 
-	return new TPromise((c, e) => {
-		const throttler = new SimpleThrottler();
+	return createCancelablePromise(token => {
+
+		once(token.onCancellationRequested)(() => {
+			logService.debug(targetPath, 'Cancelled.');
+			isCanceled = true;
+			last.cancel();
+			zipfile.close();
+		});
+
+		return new TPromise((c, e) => {
+			const throttler = new SimpleThrottler();
+
+			const readNextEntry = (token: CancellationToken) => {
+				if (token.isCancellationRequested) {
+					return;
+				}
+
+				extractedEntriesCount++;
+				zipfile.readEntry();
+			};
 
-		const readNextEntry = () => {
-			extractedEntriesCount++;
+			zipfile.once('error', e);
+			zipfile.once('close', () => last.then(() => {
+				if (isCanceled || zipfile.entryCount === extractedEntriesCount) {
+					c(null);
+				} else {
+					e(new ExtractError('Incomplete', new Error(nls.localize('incompleteExtract', "Incomplete. Found {0} of {1} entries", extractedEntriesCount, zipfile.entryCount))));
+				}
+			}, e));
 			zipfile.readEntry();
-		};
-
-		zipfile.once('error', e);
-		zipfile.once('close', () => last.then(() => {
-			if (isCanceled || zipfile.entryCount === extractedEntriesCount) {
-				c(null);
-			} else {
-				e(new ExtractError('Incomplete', new Error(nls.localize('incompleteExtract', "Incomplete. Found {0} of {1} entries", extractedEntriesCount, zipfile.entryCount))));
-			}
-		}, e));
-		zipfile.readEntry();
-		zipfile.on('entry', (entry: Entry) => {
-
-			if (isCanceled) {
-				return;
-			}
-
-			if (!options.sourcePathRegex.test(entry.fileName)) {
-				readNextEntry();
-				return;
-			}
-
-			const fileName = entry.fileName.replace(options.sourcePathRegex, '');
-
-			// directory file names end with '/'
-			if (/\/$/.test(fileName)) {
-				const targetFileName = path.join(targetPath, fileName);
-				last = mkdirp(targetFileName).then(() => readNextEntry());
-				return;
-			}
-
-			const stream = ninvoke(zipfile, zipfile.openReadStream, entry);
-			const mode = modeFromEntry(entry);
-
-			last = throttler.queue(() => stream.then(stream => extractEntry(stream, fileName, mode, targetPath, options).then(() => readNextEntry())));
+			zipfile.on('entry', (entry: Entry) => {
+
+				if (isCanceled) {
+					return;
+				}
+
+				if (!options.sourcePathRegex.test(entry.fileName)) {
+					readNextEntry(CancellationToken.None);
+					return;
+				}
+
+				const fileName = entry.fileName.replace(options.sourcePathRegex, '');
+
+				// directory file names end with '/'
+				if (/\/$/.test(fileName)) {
+					const targetFileName = path.join(targetPath, fileName);
+					last = createCancelablePromise(token => mkdirp(targetFileName, void 0, token).then(() => readNextEntry(token)));
+					return;
+				}
+
+				const stream = ninvoke(zipfile, zipfile.openReadStream, entry);
+				const mode = modeFromEntry(entry);
+
+				last = createCancelablePromise(token => throttler.queue(() => stream.then(stream => extractEntry(stream, fileName, mode, targetPath, options, token).then(() => readNextEntry(token)))));
+			});
 		});
-	}, () => {
-		logService.debug(targetPath, 'Cancelled.');
-		isCanceled = true;
-		last.cancel();
-		zipfile.close();
-	}).then(null, err => TPromise.wrapError(toExtractError(err)));
+	});
 }
 
 function openZip(zipFile: string, lazy: boolean = false): TPromise<ZipFile> {

src/vs/base/test/node/extfs/extfs.test.ts

@@ -15,8 +15,7 @@ import { isLinux, isWindows } from 'vs/base/common/platform';
 import * as uuid from 'vs/base/common/uuid';
 import * as extfs from 'vs/base/node/extfs';
 import { getPathFromAmdModule } from 'vs/base/common/amd';
-
-
+import { CancellationTokenSource } from 'vs/base/common/cancellation';
 
 const ignore = () => { };
 
@@ -564,4 +563,21 @@ suite('Extfs', () => {
 			extfs.del(parentDir, os.tmpdir(), done, ignore);
 		});
 	});
+
+	test('mkdirp cancellation', (done) => {
+		const id = uuid.generateUuid();
+		const parentDir = path.join(os.tmpdir(), 'vsctests', id);
+		const newDir = path.join(parentDir, 'extfs', id);
+
+		const source = new CancellationTokenSource();
+
+		const mkdirpPromise = extfs.mkdirp(newDir, 493, source.token);
+		source.cancel();
+
+		return mkdirpPromise.then(res => {
+			assert.equal(res, false);
+
+			extfs.del(parentDir, os.tmpdir(), done, ignore);
+		});
+	});
 });