Skip to content

Re-Introduce outdir locks to cloud assembly sources #32964

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
mrgrain opened this issue Jan 16, 2025 · 1 comment · Fixed by aws/aws-cdk-cli#344
Closed

Re-Introduce outdir locks to cloud assembly sources #32964

mrgrain opened this issue Jan 16, 2025 · 1 comment · Fixed by aws/aws-cdk-cli#344
Assignees
Labels
@aws-cdk/toolkit feature-request A feature should be added or improved. p2

Comments

@mrgrain
Copy link
Contributor

mrgrain commented Jan 16, 2025

The CLI is locking an outdir for the full course of an action. It starts when the synthesizer is run and ends when the CLI exits. Which in practice means the lock runs for a the full action.

This is slightly weird, because the action determines the locking, but the locking is local to the CxSource.

What does this mean for the Toolkit? Find a way to replicate it.

@ashishdhingra ashishdhingra added the feature-request A feature should be added or improved. label Jan 21, 2025
@mrgrain mrgrain self-assigned this Feb 11, 2025
@mrgrain mrgrain removed their assignment Mar 28, 2025
@rix0rrr rix0rrr self-assigned this Apr 7, 2025
github-merge-queue bot pushed a commit to aws/aws-cdk-cli that referenced this issue Apr 11, 2025
Cloud Assemblies are supposed to be read-locked as they are being used,
so that concurrent writers into the same directories do not trample on
the `cdk.out` directory as we are deploying it.

This is being done with the class `RWLock` which represents a read/write
lock. A lock is associated to a directory, which can have either at most
one writer or multiple readers.

This PR is a synthesis of #335 and #339.

Closes aws/aws-cdk#32964, closes #335, closes
#339.

# Design

Because we need to add the concept of a lock to a Cloud Assembly, and we
want that resource to be disposable, we introduce the class
`IReadableCloudAssembly`. This interface models both a cloud assembly
and a dispose function which releases the lock and optionally cleans up
the backing directory of the cloud assembly.

Cloud Assembly Sources (`ICloudAssemblySource.produce()`) now returns a
`IReadableCloudAssembly` with a lock associated. The factory functions
start off by acquiring write locks on the backing directory during
synthesis, which are converted to read locks after a successful
synthesis.

* All toolkit functions take an `ICloudAssemblySource`, and dispose of
the produced `IReadableCloudAssembly` after use;
* Except `synth()` now returns a `CachedCloudAssembly`. That class
implements both `IReadableCloudAssembly` (as in, it is a cloud assembly
that is locked and ready to be read), as well as `ICloudAssemblySource`
so that it can be passed into other toolkit functions. The
`CachedCloudAssembly.produce()` returns borrowed versions of
`IReadableCloudAssembly` with dispose functions that don't do anything:
only the top-level dispose actually cleans anything up. This is so that
if the result of `synth()` is passed into (multiple) other toolkit
functions, their dispose calls don't accidentally release the lock. This
could have been done with reference counting, but is currently being
done by giving out a "borrowed" Cloud Assembly which doesn't clean up at
all.
* The locking itself is managed by `ExecutionEnvironment`; this is now a
disposable object which will hold a lock that either gets cleaned up
automatically with the object, or get converted to a reader lock when
the `ExecutionEnvironment` is explicitly marked as having successfully
completed. That same change now allows us to automatically clean up
temporary directories if synthesis fails, or when the CloudAssembly in
them is disposed of.

# Supporting changes

Supporting changes in this PR, necessary to make the above work:

- `StackAssembly`, which is a helper class to query a Cloud Assembly,
used to be an `ICloudAssemblySource` but it no longer is. That
functionality isn't used anywhere.
- Rename `CachedCloudAssemblySource` to `CachedCloudAssembly` and make
`synth()` return this concretely. It makes more sense to think of this
class as a Cloud Assembly (that happens to be usable as a source), than
the reverse.
- Locks can now be released more than once; calls after the first won't
do anything. This is to prevent an `_unlock()` followed by a `dispose()`
from doing something unexpected, and it also shouldn't fail.
- Rename `ILock` => `IReadLock` to contrast it more clearly with an
`IWriteLock`.
- Remove `IdentityCloudAssemblySource`, since it fills much the same
role as `CachedCloudAssembly`.
- `ExecutionEnvironment` is now disposable, and its sync constructor has
been changed to an async factory function.
- A lot of tests that called `cx.produce()` directory now have to use an
`await using` statement to make sure the read locks are cleaned up,
otherwise they get added to the GitHub repo.

# New tests for new contracts

- Cloud Assemblies are properly disposed by all toolkit methods that
take a CloudAssemblySource
- The return value of `toolkit.synth()` can be passed to other toolkit
methods, but its contents are only disposed when the object is
explicitly disposed, not when it's passed to toolkit methods.
- When Cloud Assembly producers fail, they should leave the directory
unlocked if given, or no directory at all if they are temporary.

# Breaking change

BREAKING CHANGE: this change changes the return type of
`toolkit.synth()`: it no longer returns an arbitrary Assembly Source (by
interface), but a specific Assembly, by class, that can also be used as
a source. The return type of `ICloudAssemblySource.produce()` has been
changed to `IReadableCloudAssembly`. This will only affect consumers
with custom implementations of that interface, the factory function APIs
are unchanged.

---
By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache-2.0 license
Copy link
Contributor

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 11, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
@aws-cdk/toolkit feature-request A feature should be added or improved. p2
Projects
None yet
3 participants