Avoid blocking on macOS system certificates

CHANGELOG.md

@@ -1,6 +1,9 @@
 # Change Log
 Notable changes will be documented here.
 
+## [0.45.0]
+- Load Node.js system certificates in a worker on macOS to avoid blocking the main thread ([microsoft/vscode#333830](https://github.com/microsoft/vscode/issues/333830))
+
 ## [0.44.0]
 - Add `createProxyAuthorizationLookup` for reusable Kerberos and Basic proxy authentication handling.
 

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@vscode/proxy-agent",
-  "version": "0.44.0",
+  "version": "0.45.0",
   "description": "NodeJS http(s) agent implementation for VS Code",
   "main": "out/index.js",
   "types": "out/index.d.ts",

src/index.ts

@@ -9,11 +9,13 @@ import type * as https from 'https';
 import * as tls from 'tls';
 import * as nodeurl from 'url';
 import * as os from 'os';
+import * as path from 'path';
 import * as fs from 'fs';
 import * as cp from 'child_process';
 import * as crypto from 'crypto';
 import * as undici from 'undici';
 import * as stream from 'stream';
+import { Worker } from 'worker_threads';
 
 import { createPacProxyAgent, getProxyURLFromResolverResult, PacProxyAgent, ProxyResolveType } from './agent';
 import type { IncomingHttpHeaders } from 'undici/types/header';
@@ -209,6 +211,10 @@ export interface ResolvedProxyInfo {
 
 export function createProxyResolver(params: ProxyAgentParams) {
 	const { getProxyURL, log, proxyResolveTelemetry: proxyResolverTelemetry, env } = params;
+	if (process.platform === 'darwin' && params.loadSystemCertificatesFromNode() && (params.addCertificatesV1() || params.addCertificatesV2())) {
+		void getOrLoadAdditionalCertificates(params)
+			.catch(err => log.error('ProxyResolver#loadSystemCertificates preload error', toErrorMessage(err)));
+	}
 	let envProxy = proxyFromConfigURL(env.https_proxy || env.HTTPS_PROXY || env.http_proxy || env.HTTP_PROXY); // Not standardized.
 
 	let envNoProxy = noProxyFromEnv(env.no_proxy || env.NO_PROXY); // Not standardized.
@@ -1296,6 +1302,11 @@ export async function getOrLoadAdditionalCertificates(params: ProxyAgentParams)
 			result: undefined
 		};
 		_certs.set(loadFromNode, cert);
+		void cert.promise.catch(() => {
+			if (_certs.get(loadFromNode) === cert) {
+				_certs.delete(loadFromNode);
+			}
+		});
 	}
 	return _certs.get(loadFromNode)!.promise;
 }
@@ -1322,33 +1333,86 @@ function filterExpiredCertificates(params: CertificateParams, certs: string[]) {
 	return filtered;
 }
 
-let _systemCertificatesPromise: Promise<string[]> | undefined;
-export async function loadSystemCertificates(params: CertificateParams) {
-	if (!!params.loadSystemCertificatesFromNode?.()) { // Checking if function exists for backward compatibility.
-		const start = Date.now();
-		const systemCerts = tls.getCACertificates('system');
-		params.log.debug(`ProxyResolver#loadSystemCertificates from Node.js count (${Date.now() - start}ms)`, systemCerts.length);
-		return filterExpiredCertificates(params, systemCerts);
-	}
-	if (!_systemCertificatesPromise) {
-		_systemCertificatesPromise = (async () => {
-			try {
+const _systemCertificatesPromises = new Map<boolean, Promise<string[]>>();
+export function loadSystemCertificates(params: CertificateParams) {
+	const loadFromNode = !!params.loadSystemCertificatesFromNode?.(); // Checking if function exists for backward compatibility.
+	let systemCertificatesPromise = _systemCertificatesPromises.get(loadFromNode);
+	if (!systemCertificatesPromise) {
+		if (loadFromNode) {
+			systemCertificatesPromise = (async () => {
 				const start = Date.now();
-				const certs = await readSystemCertificates();
-				params.log.debug(`ProxyResolver#loadSystemCertificates count (${Date.now() - start}ms)`, certs.length);
+				const certs = await readNodeSystemCertificates(params.log);
+				params.log.debug(`ProxyResolver#loadSystemCertificates from Node.js count (${Date.now() - start}ms)`, certs.length);
 				return filterExpiredCertificates(params, certs);
-			} catch (err) {
-				params.log.error('ProxyResolver#loadSystemCertificates error', toErrorMessage(err));
-				return [];
+			})();
+		} else {
+			systemCertificatesPromise = (async () => {
+				try {
+					const start = Date.now();
+					const certs = await readSystemCertificates();
+					params.log.debug(`ProxyResolver#loadSystemCertificates count (${Date.now() - start}ms)`, certs.length);
+					return filterExpiredCertificates(params, certs);
+				} catch (err) {
+					params.log.error('ProxyResolver#loadSystemCertificates error', toErrorMessage(err));
+					return [];
+				}
+			})();
+		}
+		_systemCertificatesPromises.set(loadFromNode, systemCertificatesPromise);
+		void systemCertificatesPromise.catch(() => {
+			if (_systemCertificatesPromises.get(loadFromNode) === systemCertificatesPromise) {
+				_systemCertificatesPromises.delete(loadFromNode);
 			}
-		})();
+		});
 	}
-	return _systemCertificatesPromise;
+	return systemCertificatesPromise;
 }
 
 export function resetCaches() {
 	_certs.clear();
-	_systemCertificatesPromise = undefined;
+	_systemCertificatesPromises.clear();
+}
+
+async function readNodeSystemCertificates(log: Log): Promise<string[]> {
+	if (process.platform !== 'darwin') {
+		return tls.getCACertificates('system');
+	}
+
+	try {
+		return await new Promise<string[]>((resolve, reject) => {
+			log.debug('ProxyResolver#loadSystemCertificates starting worker');
+			const workerStart = Date.now();
+			const worker = new Worker(path.join(__dirname, 'systemCertificatesWorker.js'), { name: 'System certificate loader' });
+			worker.unref();
+			let settled = false;
+			worker.once('message', (certs: unknown) => {
+				settled = true;
+				if (!isStringArray(certs)) {
+					void worker.terminate();
+					reject(new Error('System certificate worker returned an invalid result'));
+					return;
+				}
+				resolve(certs);
+			});
+			worker.once('error', err => {
+				settled = true;
+				reject(err);
+			});
+			worker.once('exit', code => {
+				log.debug(`ProxyResolver#loadSystemCertificates worker exited (${Date.now() - workerStart}ms)`, code);
+				if (!settled) {
+					reject(new Error(`System certificate worker exited with code ${code}`));
+				}
+			});
+		});
+	} catch (err) {
+		log.warn('ProxyResolver#loadSystemCertificates worker failed, falling back to main thread', toErrorMessage(err));
+		return tls.getCACertificates('system');
+	}
+}
+
+function isStringArray(value: unknown): value is string[] {
+	return Array.isArray(value) && value.every(item => typeof item === 'string');
 }
 
 async function readSystemCertificates(): Promise<string[]> {
@@ -1463,5 +1527,3 @@ export function toLogString(args: any[]) {
 			return value;
 		})).join(', ')}]`;
 }
-
-

src/systemCertificatesWorker.ts

@@ -0,0 +1,13 @@
+/*---------------------------------------------------------------------------------------------
+ *  Copyright (c) Microsoft Corporation. All rights reserved.
+ *  Licensed under the MIT License. See License.txt in the project root for license information.
+ *--------------------------------------------------------------------------------------------*/
+
+import * as tls from 'tls';
+import { parentPort } from 'worker_threads';
+
+if (!parentPort) {
+	throw new Error('System certificate worker must run in a worker thread');
+}
+
+parentPort.postMessage(tls.getCACertificates('system'));

tests/src/certificateLoading.test.ts

@@ -0,0 +1,39 @@
+import * as assert from 'assert';
+import { getOrLoadAdditionalCertificates, LogLevel, ProxyAgentParams, resetCaches } from '../../src';
+
+function createParams(loadAdditionalCertificates: () => Promise<string[]>): ProxyAgentParams {
+	const noop = () => { };
+	return {
+		resolveProxy: async () => undefined,
+		getProxyURL: () => undefined,
+		getProxySupport: () => 'override',
+		isAdditionalFetchSupportEnabled: () => true,
+		isWebSocketPatchEnabled: () => true,
+		addCertificatesV1: () => true,
+		addCertificatesV2: () => false,
+		loadSystemCertificatesFromNode: () => true,
+		loadAdditionalCertificates,
+		log: { trace: noop, debug: noop, info: noop, warn: noop, error: noop },
+		getLogLevel: () => LogLevel.Off,
+		proxyResolveTelemetry: noop,
+		isUseHostProxyEnabled: () => true,
+		env: {},
+	};
+}
+
+describe('certificate loading', function () {
+	afterEach(() => resetCaches());
+
+	it('retries loading additional certificates after a failure', async function () {
+		let attempts = 0;
+		const params = createParams(async () => {
+			if (++attempts === 1) {
+				throw new Error('Certificate loading failed');
+			}
+			return ['certificate'];
+		});
+
+		await assert.rejects(getOrLoadAdditionalCertificates(params), /Certificate loading failed/);
+		assert.deepStrictEqual(await getOrLoadAdditionalCertificates(params), ['certificate']);
+	});
+});

tests/src/resolveProxyByURL.test.ts

@@ -1,5 +1,5 @@
 import * as assert from 'assert';
-import { createProxyResolver, LogLevel, ProxyAgentParams } from '../../src';
+import { createProxyResolver, getOrLoadAdditionalCertificates, loadSystemCertificates, Log, LogLevel, ProxyAgentParams, resetCaches } from '../../src';
 
 function createParams(overrides: Partial<ProxyAgentParams>): ProxyAgentParams {
 	const noop = () => { };
@@ -24,6 +24,45 @@ function createParams(overrides: Partial<ProxyAgentParams>): ProxyAgentParams {
 }
 
 describe('resolveProxyByURL', function () {
+	it('preloads Node.js system certificates on macOS', async function () {
+		if (process.platform !== 'darwin') {
+			this.skip();
+		}
+
+		resetCaches();
+		const debugMessages: string[] = [];
+		const log: Log = {
+			trace: () => { },
+			debug: message => debugMessages.push(message),
+			info: () => { },
+			warn: () => { },
+			error: () => { },
+		};
+		const params = createParams({
+			addCertificatesV1: () => true,
+			loadSystemCertificatesFromNode: () => true,
+			loadAdditionalCertificates: () => loadSystemCertificates({
+				loadSystemCertificatesFromNode: () => true,
+				log,
+			}),
+			log,
+		});
+		try {
+			createProxyResolver(params);
+			const startedDuringResolverCreation = debugMessages.includes('ProxyResolver#loadSystemCertificates starting worker');
+			await getOrLoadAdditionalCertificates(params);
+			assert.deepStrictEqual({
+				startedDuringResolverCreation,
+				finishedLoadingSystemCertificates: debugMessages.some(message => message.startsWith('ProxyResolver#loadSystemCertificates from Node.js count')),
+			}, {
+				startedDuringResolverCreation: true,
+				finishedLoadingSystemCertificates: true,
+			});
+		} finally {
+			resetCaches();
+		}
+	});
+
 	it('reports localhost as a direct connection', async function () {
 		const { resolveProxyByURL } = createProxyResolver(createParams({}));
 		assert.deepStrictEqual(await resolveProxyByURL('http://localhost:3000/'), {