Skip to content

Commit 8c54bba

Browse files
vladimavladima
authored
use separate process to probe if drive is safe to watch (microsoft#14098)
use dedicated process to determine if it is safe to watch folders
1 parent c90040e commit 8c54bba

5 files changed

Lines changed: 135 additions & 15 deletions

File tree

‎Jakefile.js‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ var servicesDirectory = "src/services/";
1515
var serverDirectory = "src/server/";
1616
var typingsInstallerDirectory = "src/server/typingsInstaller";
1717
var cancellationTokenDirectory = "src/server/cancellationToken";
18+
var watchGuardDirectory = "src/server/watchGuard";
1819
var harnessDirectory = "src/harness/";
1920
var libraryDirectory = "src/lib/";
2021
var scriptsDirectory = "scripts/";
@@ -80,6 +81,7 @@ var compilerSources = filesFromConfig("./src/compiler/tsconfig.json");
8081
var servicesSources = filesFromConfig("./src/services/tsconfig.json");
8182
var cancellationTokenSources = filesFromConfig(path.join(serverDirectory, "cancellationToken/tsconfig.json"));
8283
var typingsInstallerSources = filesFromConfig(path.join(serverDirectory, "typingsInstaller/tsconfig.json"));
84+
var watchGuardSources = filesFromConfig(path.join(serverDirectory, "watchGuard/tsconfig.json"));
8385
var serverSources = filesFromConfig(path.join(serverDirectory, "tsconfig.json"))
8486
var languageServiceLibrarySources = filesFromConfig(path.join(serverDirectory, "tsconfig.library.json"));
8587

@@ -570,8 +572,11 @@ compileFile(cancellationTokenFile, cancellationTokenSources, [builtLocalDirector
570572
var typingsInstallerFile = path.join(builtLocalDirectory, "typingsInstaller.js");
571573
compileFile(typingsInstallerFile, typingsInstallerSources, [builtLocalDirectory].concat(typingsInstallerSources), /*prefixes*/ [copyright], /*useBuiltCompiler*/ true, { outDir: builtLocalDirectory, noOutFile: false });
572574

575+
var watchGuardFile = path.join(builtLocalDirectory, "watchGuard.js");
576+
compileFile(watchGuardFile, watchGuardSources, [builtLocalDirectory].concat(watchGuardSources), /*prefixes*/ [copyright], /*useBuiltCompiler*/ true, { outDir: builtLocalDirectory, noOutFile: false });
577+
573578
var serverFile = path.join(builtLocalDirectory, "tsserver.js");
574-
compileFile(serverFile, serverSources, [builtLocalDirectory, copyright, cancellationTokenFile, typingsInstallerFile].concat(serverSources), /*prefixes*/ [copyright], /*useBuiltCompiler*/ true, { types: ["node"], preserveConstEnums: true });
579+
compileFile(serverFile, serverSources, [builtLocalDirectory, copyright, cancellationTokenFile, typingsInstallerFile, watchGuardFile].concat(serverSources), /*prefixes*/ [copyright], /*useBuiltCompiler*/ true, { types: ["node"], preserveConstEnums: true });
575580
var tsserverLibraryFile = path.join(builtLocalDirectory, "tsserverlibrary.js");
576581
var tsserverLibraryDefinitionFile = path.join(builtLocalDirectory, "tsserverlibrary.d.ts");
577582
compileFile(
@@ -665,7 +670,7 @@ task("generate-spec", [specMd]);
665670
// Makes a new LKG. This target does not build anything, but errors if not all the outputs are present in the built/local directory
666671
desc("Makes a new LKG out of the built js files");
667672
task("LKG", ["clean", "release", "local"].concat(libraryTargets), function () {
668-
var expectedFiles = [tscFile, servicesFile, serverFile, nodePackageFile, nodeDefinitionsFile, standaloneDefinitionsFile, tsserverLibraryFile, tsserverLibraryDefinitionFile, cancellationTokenFile, typingsInstallerFile, buildProtocolDts].concat(libraryTargets);
673+
var expectedFiles = [tscFile, servicesFile, serverFile, nodePackageFile, nodeDefinitionsFile, standaloneDefinitionsFile, tsserverLibraryFile, tsserverLibraryDefinitionFile, cancellationTokenFile, typingsInstallerFile, buildProtocolDts, watchGuardFile].concat(libraryTargets);
669674
var missingFiles = expectedFiles.filter(function (f) {
670675
return !fs.existsSync(f);
671676
});

‎src/compiler/sys.ts‎

Lines changed: 20 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,21 @@ namespace ts {
5959
declare var global: any;
6060
declare var __filename: string;
6161

62+
export function getNodeMajorVersion() {
63+
if (typeof process === "undefined") {
64+
return undefined;
65+
}
66+
const version: string = process.version;
67+
if (!version) {
68+
return undefined;
69+
}
70+
const dot = version.indexOf(".");
71+
if (dot === -1) {
72+
return undefined;
73+
}
74+
return parseInt(version.substring(1, dot));
75+
}
76+
6277
declare class Enumerator {
6378
public atEnd(): boolean;
6479
public moveNext(): boolean;
@@ -315,9 +330,8 @@ namespace ts {
315330
}
316331
const watchedFileSet = createWatchedFileSet();
317332

318-
function isNode4OrLater(): boolean {
319-
return parseInt(process.version.charAt(1)) >= 4;
320-
}
333+
const nodeVersion = getNodeMajorVersion();
334+
const isNode4OrLater = nodeVersion >= 4;
321335

322336
function isFileSystemCaseSensitive(): boolean {
323337
// win32\win64 are case insensitive platforms
@@ -485,14 +499,12 @@ namespace ts {
485499
// Node 4.0 `fs.watch` function supports the "recursive" option on both OSX and Windows
486500
// (ref: https://github.com/nodejs/node/pull/2649 and https://github.com/Microsoft/TypeScript/issues/4643)
487501
let options: any;
488-
if (!directoryExists(directoryName) || (isUNCPath(directoryName) && process.platform === "win32")) {
489-
// do nothing if either
490-
// - target folder does not exist
491-
// - this is UNC path on Windows (https://github.com/Microsoft/TypeScript/issues/13874)
502+
if (!directoryExists(directoryName)) {
503+
// do nothing if target folder does not exist
492504
return noOpFileWatcher;
493505
}
494506

495-
if (isNode4OrLater() && (process.platform === "win32" || process.platform === "darwin")) {
507+
if (isNode4OrLater && (process.platform === "win32" || process.platform === "darwin")) {
496508
options = { persistent: true, recursive: !!recursive };
497509
}
498510
else {
@@ -512,10 +524,6 @@ namespace ts {
512524
};
513525
}
514526
);
515-
516-
function isUNCPath(s: string): boolean {
517-
return s.length > 2 && s.charCodeAt(0) === CharacterCodes.slash && s.charCodeAt(1) === CharacterCodes.slash;
518-
}
519527
},
520528
resolvePath: function(path: string): string {
521529
return _path.resolve(path);

‎src/server/server.ts‎

Lines changed: 79 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ namespace ts.server {
1212

1313
const childProcess: {
1414
fork(modulePath: string, args: string[], options?: { execArgv: string[], env?: MapLike<string> }): NodeChildProcess;
15+
execFileSync(file: string, args: string[], options: { stdio: "ignore", env: MapLike<string> }): string | Buffer;
1516
} = require("child_process");
1617

1718
const os: {
@@ -59,7 +60,7 @@ namespace ts.server {
5960

6061
interface NodeChildProcess {
6162
send(message: any, sendHandle?: any): void;
62-
on(message: "message", f: (m: any) => void): void;
63+
on(message: "message" | "exit", f: (m: any) => void): void;
6364
kill(): void;
6465
pid: number;
6566
}
@@ -576,7 +577,84 @@ namespace ts.server {
576577
}
577578
}
578579

580+
function extractWatchDirectoryCacheKey(path: string, currentDriveKey: string) {
581+
path = normalizeSlashes(path);
582+
if (isUNCPath(path)) {
583+
// UNC path: extract server name
584+
// //server/location
585+
// ^ <- from 0 to this position
586+
const firstSlash = path.indexOf(directorySeparator, 2);
587+
return firstSlash !== -1 ? path.substring(0, firstSlash).toLowerCase() : path;
588+
}
589+
const rootLength = getRootLength(path);
590+
if (rootLength === 0) {
591+
// relative path - assume file is on the current drive
592+
return currentDriveKey;
593+
}
594+
if (path.charCodeAt(1) === CharacterCodes.colon && path.charCodeAt(2) === CharacterCodes.slash) {
595+
// rooted path that starts with c:/... - extract drive letter
596+
return path.charAt(0).toLowerCase();
597+
}
598+
if (path.charCodeAt(0) === CharacterCodes.slash && path.charCodeAt(1) !== CharacterCodes.slash) {
599+
// rooted path that starts with slash - /somename - use key for current drive
600+
return currentDriveKey;
601+
}
602+
// do not cache any other cases
603+
return undefined;
604+
}
605+
606+
function isUNCPath(s: string): boolean {
607+
return s.length > 2 && s.charCodeAt(0) === CharacterCodes.slash && s.charCodeAt(1) === CharacterCodes.slash;
608+
}
609+
579610
const sys = <ServerHost>ts.sys;
611+
// use watchGuard process on Windows when node version is 4 or later
612+
const useWatchGuard = process.platform === "win32" && getNodeMajorVersion() >= 4;
613+
if (useWatchGuard) {
614+
const currentDrive = extractWatchDirectoryCacheKey(sys.resolvePath(sys.getCurrentDirectory()), /*currentDriveKey*/ undefined);
615+
const statusCache = createMap<boolean>();
616+
const originalWatchDirectory = sys.watchDirectory;
617+
sys.watchDirectory = function (path: string, callback: DirectoryWatcherCallback, recursive?: boolean): FileWatcher {
618+
const cacheKey = extractWatchDirectoryCacheKey(path, currentDrive);
619+
let status = cacheKey && statusCache.get(cacheKey);
620+
if (status === undefined) {
621+
if (logger.hasLevel(LogLevel.verbose)) {
622+
logger.info(`${cacheKey} for path ${path} not found in cache...`);
623+
}
624+
try {
625+
const args = [combinePaths(__dirname, "watchGuard.js"), path];
626+
if (logger.hasLevel(LogLevel.verbose)) {
627+
logger.info(`Starting ${process.execPath} with args ${JSON.stringify(args)}`);
628+
}
629+
childProcess.execFileSync(process.execPath, args, { stdio: "ignore", env: { "ELECTRON_RUN_AS_NODE": "1" } });
630+
status = true;
631+
if (logger.hasLevel(LogLevel.verbose)) {
632+
logger.info(`WatchGuard for path ${path} returned: OK`);
633+
}
634+
}
635+
catch (e) {
636+
status = false;
637+
if (logger.hasLevel(LogLevel.verbose)) {
638+
logger.info(`WatchGuard for path ${path} returned: ${e.message}`);
639+
}
640+
}
641+
if (cacheKey) {
642+
statusCache.set(cacheKey, status);
643+
}
644+
}
645+
else if (logger.hasLevel(LogLevel.verbose)) {
646+
logger.info(`watchDirectory for ${path} uses cached drive information.`);
647+
}
648+
if (status) {
649+
// this drive is safe to use - call real 'watchDirectory'
650+
return originalWatchDirectory.call(sys, path, callback, recursive);
651+
}
652+
else {
653+
// this drive is unsafe - return no-op watcher
654+
return { close() { } };
655+
}
656+
}
657+
}
580658

581659
// Override sys.write because fs.writeSync is not reliable on Node 4
582660
sys.write = (s: string) => writeMessage(new Buffer(s, "utf8"));

‎src/server/watchGuard/tsconfig.json‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
{
2+
"extends": "../../tsconfig-base",
3+
"compilerOptions": {
4+
"removeComments": true,
5+
"outFile": "../../../built/local/watchGuard.js"
6+
},
7+
"files": [
8+
"watchGuard.ts"
9+
]
10+
}

‎src/server/watchGuard/watchGuard.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
/// <reference types="node" />
2+
3+
if (process.argv.length < 3) {
4+
process.exit(1);
5+
}
6+
const directoryName = process.argv[2];
7+
const fs: { watch(directoryName: string, options: any, callback: () => {}): any } = require("fs");
8+
// main reason why we need separate process to check if it is safe to watch some path
9+
// is to guard against crashes that cannot be intercepted with protected blocks and
10+
// code in tsserver already can handle normal cases, like non-existing folders.
11+
// This means that here we treat any result (success or exception) from fs.watch as success since it does not tear down the process.
12+
// The only case that should be considered as failure - when watchGuard process crashes.
13+
try {
14+
const watcher = fs.watch(directoryName, { recursive: true }, () => ({}))
15+
watcher.close();
16+
}
17+
catch (_e) {
18+
}
19+
process.exit(0);

0 commit comments

Comments
 (0)