Skip to content

Commit

Permalink
fix(cli): release outdir lock when synth fails (#30874)
Browse files Browse the repository at this point in the history
### 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*
  • Loading branch information
misterjoshua committed Sep 16, 2024
1 parent e220e90 commit b6ad97f
Show file tree
Hide file tree
Showing 2 changed files with 46 additions and 21 deletions.
47 changes: 26 additions & 21 deletions packages/aws-cdk/lib/api/cxapp/exec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<void>((ok, fail) => {
Expand Down
20 changes: 20 additions & 0 deletions packages/aws-cdk/test/api/exec.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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: [],
Expand Down

0 comments on commit b6ad97f

Please sign in to comment.