From 5cb3f2e4e4b1888009f60ec79e503453ed81aefb Mon Sep 17 00:00:00 2001 From: swapnil-nagar Date: Fri, 21 Aug 2026 15:21:38 -0700 Subject: [PATCH] Fix specialization double-start race Preserve initialized app state when worker init and environment reload target the same directory, and exclude node_modules from entry-point globs. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: eff15798-612c-4121-a6d4-ec5e093bff1f --- .../FunctionEnvironmentReloadHandler.ts | 24 ++++++---- src/startApp.ts | 7 ++- .../FunctionEnvironmentReloadHandler.test.ts | 48 +++++++++++++++++-- test/eventHandlers/WorkerInitHandler.test.ts | 23 +++++++++ .../testApp/src/dependencyEntry.js | 2 + .../testApp/src/registerV4Function.js | 7 +++ 6 files changed, 96 insertions(+), 15 deletions(-) create mode 100644 test/eventHandlers/testApp/src/dependencyEntry.js create mode 100644 test/eventHandlers/testApp/src/registerV4Function.js diff --git a/src/eventHandlers/FunctionEnvironmentReloadHandler.ts b/src/eventHandlers/FunctionEnvironmentReloadHandler.ts index 8fdb9e7a..d1256451 100644 --- a/src/eventHandlers/FunctionEnvironmentReloadHandler.ts +++ b/src/eventHandlers/FunctionEnvironmentReloadHandler.ts @@ -27,6 +27,11 @@ export class FunctionEnvironmentReloadHandler extends EventHandler< } async handleEvent(msg: rpc.IFunctionEnvironmentReloadRequest): Promise { + const functionAppDirectoryUnchanged = + !!worker.app.functionAppDirectory && + !!msg.functionAppDirectory && + isPathEqual(worker.app.functionAppDirectory, msg.functionAppDirectory); + if (!msg.functionAppDirectory) { worker.log({ message: `FunctionEnvironmentReload functionAppDirectory is not defined`, @@ -35,11 +40,7 @@ export class FunctionEnvironmentReloadHandler extends EventHandler< }); } - if ( - worker.app.functionAppDirectory && - msg.functionAppDirectory && - isPathEqual(worker.app.functionAppDirectory, msg.functionAppDirectory) - ) { + if (functionAppDirectoryUnchanged) { worker.log({ message: `FunctionEnvironmentReload functionAppDirectory has not changed`, level: rpc.RpcLog.Level.Debug, @@ -47,7 +48,10 @@ export class FunctionEnvironmentReloadHandler extends EventHandler< }); } - worker.resetApp(msg.functionAppDirectory); + // Preserve registrations when specialization sends both requests for the same app; its modules are already cached. + if (!functionAppDirectoryUnchanged) { + worker.resetApp(msg.functionAppDirectory); + } const response = this.getDefaultResponse(msg); @@ -72,9 +76,11 @@ export class FunctionEnvironmentReloadHandler extends EventHandler< logCategory: rpc.RpcLog.RpcLogCategory.System, }); process.chdir(msg.functionAppDirectory); - await startApp(msg.functionAppDirectory); - // model info may have changed, so we need to update this - response.workerMetadata = getWorkerMetadata(); + if (!functionAppDirectoryUnchanged) { + await startApp(msg.functionAppDirectory); + // model info may have changed, so we need to update this + response.workerMetadata = getWorkerMetadata(); + } } validateNodeVersion(process.version); diff --git a/src/startApp.ts b/src/startApp.ts index 83e3ce5f..bc2af01f 100644 --- a/src/startApp.ts +++ b/src/startApp.ts @@ -18,7 +18,7 @@ import path = require('path'); * 1. The worker can start in "normal" mode, meaning `workerInitRequest` will reference the user's app * 2. The worker can start in "placeholder" mode, meaning `workerInitRequest` will reference a dummy app to "warm up" the worker and `functionEnvironmentReloadRequest` will be sent with the user's actual app. * This process is called worker specialization and it helps with cold start times. - * The dummy app should never have actual startup code, so it should be safe to call `startApp` twice in this case + * The app is only started after specialization if the directory differs from the one supplied during worker init. * Worker specialization happens only once, so we don't need to worry about cleaning up resources from previous `functionEnvironmentReloadRequest`s. */ export async function startApp(functionAppDirectory: string): Promise { @@ -61,7 +61,10 @@ async function loadEntryPointFile(functionAppDirectory: string): Promise { if (entryPointPattern) { let currentFile: string | undefined = undefined; try { - const files = await globby(entryPointPattern, { cwd: functionAppDirectory }); + const files = await globby(entryPointPattern, { + cwd: functionAppDirectory, + ignore: ['**/node_modules/**'], + }); if (files.length === 0) { let message: string = globby.hasMagic(entryPointPattern, { cwd: functionAppDirectory }) ? 'Found zero files matching the supplied pattern' diff --git a/test/eventHandlers/FunctionEnvironmentReloadHandler.test.ts b/test/eventHandlers/FunctionEnvironmentReloadHandler.test.ts index 6fc3783e..cd2279b7 100644 --- a/test/eventHandlers/FunctionEnvironmentReloadHandler.test.ts +++ b/test/eventHandlers/FunctionEnvironmentReloadHandler.test.ts @@ -192,9 +192,15 @@ describe('FunctionEnvironmentReloadHandler', () => { process.chdir(cwd); }); - it('reloads package.json', async () => { + it('preserves the initialized app when the function app directory is unchanged', async () => { const oldPackageJson = { type: 'module', hello: 'world' }; await fs.writeFile(testPackageJsonPath, JSON.stringify(oldPackageJson)); + stream.addTestMessage(msg.init.request(testAppPath)); + await stream.assertCalledWith(msg.init.receivedRequestLog, msg.init.nodeVersionLog(), msg.init.response); + expect(worker.app.packageJson).to.deep.equal(oldPackageJson); + + const newPackageJson = { type: 'commonjs', notHello: 'notWorld' }; + await fs.writeFile(testPackageJsonPath, JSON.stringify(newPackageJson)); stream.addTestMessage({ requestId: 'testReqId', functionEnvironmentReloadRequest: { @@ -202,15 +208,29 @@ describe('FunctionEnvironmentReloadHandler', () => { }, }); await stream.assertCalledWith( + msg.envReload.funcAppDirNotChanged, msg.envReload.reloadEnvVarsLog(0), msg.envReload.changingCwdLog(testAppPath), msg.envReload.nodeVersionLog(), msg.envReload.response ); expect(worker.app.packageJson).to.deep.equal(oldPackageJson); + }); + + it('preserves v4 registrations when init and reload reference the same app', async () => { + const fileName = 'registerV4Function.js'; + const fileSubpath = await setTestAppMainField(fileName); + + stream.addTestMessage(msg.init.request(testAppPath)); + await stream.assertCalledWith( + msg.init.receivedRequestLog, + msg.loadingEntryPoint(fileSubpath), + msg.infoLog('Setting Node.js programming model to "@azure/functions" version "4.12.0"'), + msg.loadedEntryPoint(fileSubpath), + msg.init.nodeVersionLog(), + msg.init.response + ); - const newPackageJson = { type: 'commonjs', notHello: 'notWorld' }; - await fs.writeFile(testPackageJsonPath, JSON.stringify(newPackageJson)); stream.addTestMessage({ requestId: 'testReqId', functionEnvironmentReloadRequest: { @@ -224,7 +244,27 @@ describe('FunctionEnvironmentReloadHandler', () => { msg.envReload.nodeVersionLog(), msg.envReload.response ); - expect(worker.app.packageJson).to.deep.equal(newPackageJson); + + expect(worker.app.programmingModel?.version).to.equal('4.12.0'); + expect(worker.app.isUsingWorkerIndexing).to.be.true; + + stream.addTestMessage(msg.indexing.request); + await stream.assertCalledWith( + msg.indexing.receivedRequestLog, + msg.indexing.response( + [ + { + bindings: {}, + directory: testAppSrcPath, + functionId: 'testFunc', + name: 'testFunc', + rawBindings: [], + scriptFile: fileName, + }, + ], + false + ) + ); }); it('loads package.json (placeholder scenario)', async () => { diff --git a/test/eventHandlers/WorkerInitHandler.test.ts b/test/eventHandlers/WorkerInitHandler.test.ts index 60c247de..f4218657 100644 --- a/test/eventHandlers/WorkerInitHandler.test.ts +++ b/test/eventHandlers/WorkerInitHandler.test.ts @@ -5,6 +5,7 @@ import 'mocha'; import * as coreTypes from '@azure/functions-core'; import { expect } from 'chai'; import * as fs from 'fs/promises'; +import * as path from 'path'; import { logColdStartWarning } from '../../src/eventHandlers/WorkerInitHandler'; import { isNode20Plus } from '../../src/utils/util'; import { worker } from '../../src/WorkerContext'; @@ -124,6 +125,28 @@ describe('WorkerInitHandler', () => { ); }); + it('excludes node_modules from entry point globs', async () => { + const fileSubpath = 'src/dependencyEntry.js'; + await fs.writeFile(testPackageJsonPath, JSON.stringify({ main: '**/dependencyEntry.js' })); + const nodeModulesDirectory = path.join(testAppPath, 'node_modules'); + const dependencyDirectory = path.join(nodeModulesDirectory, 'test-dependency'); + await fs.mkdir(dependencyDirectory, { recursive: true }); + await fs.writeFile(path.join(dependencyDirectory, 'dependencyEntry.js'), ''); + + try { + stream.addTestMessage(msg.init.request(testAppPath)); + await stream.assertCalledWith( + msg.init.receivedRequestLog, + msg.loadingEntryPoint(fileSubpath), + msg.loadedEntryPoint(fileSubpath), + msg.init.nodeVersionLog(), + msg.init.response + ); + } finally { + await fs.rm(nodeModulesDirectory, { recursive: true, force: true }); + } + }); + for (const rfpValue of ['1', 'https://url']) { it(`Skips warn for long load time if rfp already set to ${rfpValue}`, async () => { const fileSubpath = await setTestAppMainField('longLoad.js'); diff --git a/test/eventHandlers/testApp/src/dependencyEntry.js b/test/eventHandlers/testApp/src/dependencyEntry.js new file mode 100644 index 00000000..00e25a45 --- /dev/null +++ b/test/eventHandlers/testApp/src/dependencyEntry.js @@ -0,0 +1,2 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the MIT License. diff --git a/test/eventHandlers/testApp/src/registerV4Function.js b/test/eventHandlers/testApp/src/registerV4Function.js new file mode 100644 index 00000000..8f9b7ad5 --- /dev/null +++ b/test/eventHandlers/testApp/src/registerV4Function.js @@ -0,0 +1,7 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the MIT License. + +const func = require('@azure/functions-core'); + +func.setProgrammingModel({ name: '@azure/functions', version: '4.12.0' }); +func.registerFunction({ name: 'testFunc', bindings: [] }, () => {});