Fix specialization double-start race

src/eventHandlers/FunctionEnvironmentReloadHandler.ts

@@ -27,6 +27,11 @@ export class FunctionEnvironmentReloadHandler extends EventHandler<
     }
 
     async handleEvent(msg: rpc.IFunctionEnvironmentReloadRequest): Promise<rpc.IFunctionEnvironmentReloadResponse> {
+        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,19 +40,18 @@ 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,
                 logCategory: rpc.RpcLog.RpcLogCategory.System,
             });
         }
 
-        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);

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<void> {
@@ -61,7 +61,10 @@ async function loadEntryPointFile(functionAppDirectory: string): Promise<void> {
     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'

test/eventHandlers/FunctionEnvironmentReloadHandler.test.ts

@@ -192,25 +192,45 @@ 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: {
                 functionAppDirectory: testAppPath,
             },
         });
         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 () => {

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');

test/eventHandlers/testApp/src/dependencyEntry.js

@@ -0,0 +1,2 @@
+// Copyright (c) .NET Foundation. All rights reserved.
+// Licensed under the MIT License.

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: [] }, () => {});