From b6ad97f4b4e1c185ddc53f60e15b0dabd8022694 Mon Sep 17 00:00:00 2001 From: Josh Kellendonk Date: Mon, 16 Sep 2024 11:04:43 -0600 Subject: [PATCH] fix(cli): release outdir lock when synth fails (#30874) ### Issue # (if applicable) Closes #27864 ### Reason for this change When using cdk watch mode, a synth failure causes the CDK CLI to no longer deploy changes. The CDK CLI must be restarted to resume watch mode. The cause of the issue is that CDK CLI never releases the outdir write lock if synthing fails, so subsequent attempts to exec the user's app cannot acquire the outdir writer lock. ### Description of changes I added a try/catch that releases the outdir writer lock & rethrows the error when a synth fails. ### Description of how you validated changes I added a unit test. I also ran the modified cdk cli on a project of my own and simulated the failure of a synth to see whether the issue was resolved, and it is. ### Checklist - [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- packages/aws-cdk/lib/api/cxapp/exec.ts | 47 ++++++++++++++------------ packages/aws-cdk/test/api/exec.test.ts | 20 +++++++++++ 2 files changed, 46 insertions(+), 21 deletions(-) diff --git a/packages/aws-cdk/lib/api/cxapp/exec.ts b/packages/aws-cdk/lib/api/cxapp/exec.ts index 4069d7a660143..31f2fca029dd9 100644 --- a/packages/aws-cdk/lib/api/cxapp/exec.ts +++ b/packages/aws-cdk/lib/api/cxapp/exec.ts @@ -58,37 +58,42 @@ export async function execProgram(aws: SdkProvider, config: Configuration): Prom debug('outdir:', outdir); env[cxapi.OUTDIR_ENV] = outdir; - // Acquire a read lock on the output directory + // Acquire a lock on the output directory const writerLock = await new RWLock(outdir).acquireWrite(); - // Send version information - env[cxapi.CLI_ASM_VERSION_ENV] = cxschema.Manifest.version(); - env[cxapi.CLI_VERSION_ENV] = versionNumber(); + try { + // Send version information + env[cxapi.CLI_ASM_VERSION_ENV] = cxschema.Manifest.version(); + env[cxapi.CLI_VERSION_ENV] = versionNumber(); - debug('env:', env); + debug('env:', env); - const envVariableSizeLimit = os.platform() === 'win32' ? 32760 : 131072; - const [smallContext, overflow] = splitBySize(context, spaceAvailableForContext(env, envVariableSizeLimit)); + const envVariableSizeLimit = os.platform() === 'win32' ? 32760 : 131072; + const [smallContext, overflow] = splitBySize(context, spaceAvailableForContext(env, envVariableSizeLimit)); - // Store the safe part in the environment variable - env[cxapi.CONTEXT_ENV] = JSON.stringify(smallContext); + // Store the safe part in the environment variable + env[cxapi.CONTEXT_ENV] = JSON.stringify(smallContext); - // If there was any overflow, write it to a temporary file - let contextOverflowLocation; - if (Object.keys(overflow ?? {}).length > 0) { - const contextDir = await fs.mkdtemp(path.join(os.tmpdir(), 'cdk-context')); - contextOverflowLocation = path.join(contextDir, 'context-overflow.json'); - fs.writeJSONSync(contextOverflowLocation, overflow); - env[cxapi.CONTEXT_OVERFLOW_LOCATION_ENV] = contextOverflowLocation; - } + // If there was any overflow, write it to a temporary file + let contextOverflowLocation; + if (Object.keys(overflow ?? {}).length > 0) { + const contextDir = await fs.mkdtemp(path.join(os.tmpdir(), 'cdk-context')); + contextOverflowLocation = path.join(contextDir, 'context-overflow.json'); + fs.writeJSONSync(contextOverflowLocation, overflow); + env[cxapi.CONTEXT_OVERFLOW_LOCATION_ENV] = contextOverflowLocation; + } - await exec(commandLine.join(' ')); + await exec(commandLine.join(' ')); - const assembly = createAssembly(outdir); + const assembly = createAssembly(outdir); - contextOverflowCleanup(contextOverflowLocation, assembly); + contextOverflowCleanup(contextOverflowLocation, assembly); - return { assembly, lock: await writerLock.convertToReaderLock() }; + return { assembly, lock: await writerLock.convertToReaderLock() }; + } catch (e) { + await writerLock.release(); + throw e; + } async function exec(commandAndArgs: string) { return new Promise((ok, fail) => { diff --git a/packages/aws-cdk/test/api/exec.test.ts b/packages/aws-cdk/test/api/exec.test.ts index a8c32aed06d8a..f3061678e3d77 100644 --- a/packages/aws-cdk/test/api/exec.test.ts +++ b/packages/aws-cdk/test/api/exec.test.ts @@ -12,6 +12,7 @@ import { Configuration } from '../../lib/settings'; import { testAssembly } from '../util'; import { mockSpawn } from '../util/mock-child_process'; import { MockSdkProvider } from '../util/mock-sdk'; +import { RWLock } from '../../lib/api/util/rwlock'; let sdkProvider: MockSdkProvider; let config: Configuration; @@ -234,6 +235,25 @@ test('cli does not throw when the `build` script succeeds', async () => { await lock.release(); }, TEN_SECOND_TIMEOUT); +test('cli releases the outdir lock when execProgram throws', async () => { + // GIVEN + config.settings.set(['app'], 'cloud-executable'); + mockSpawn({ + commandLine: 'fake-command', + exitCode: 127, + }); + + // WHEN + await expect(execProgram(sdkProvider, config)).rejects.toThrow(); + + const output = config.settings.get(['output']); + expect(output).toBeDefined(); + + // check that the lock is released + const lock = await new RWLock(output).acquireWrite(); + await lock.release(); +}); + function writeOutputAssembly() { const asm = testAssembly({ stacks: [],