fix(cdk-assets): concurrent asset building may occurs ENOENT: no such file or directory, unlink
#25293
Conversation
…tly by ensuring unique file names during zip file generation.
|
|
github-actions
bot
added
p2
beginning-contributor
There was a problem hiding this comment.
The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.
A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.
tockn
changed the title
ENOENT: no such file or directory, unlink ENOENT: no such file or directory, unlink
|
Exemption Request |
aws-cdk-automation
added
the
pr-linter/exemption-requested
|
@rix0rrr Hi. Thank you for the maintenance work you've done. Would it be possible for you to review this pr? |
aws-cdk-automation
added
the
pr/needs-community-review
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
|
This PR has been in the CHANGES REQUESTED state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week. |
|
@corymhall Thank you for the maintenance. Would it be possible for you to review this for me? |
|
@corymhall Is there any updates? 🙇♂️ |
corymhall
removed
the
pr/needs-community-review
|
The pull request linter fails with the following errors: PRs must pass status checks before we can provide a meaningful review. If you would like to request an exemption from the status checks or clarification on feedback, please leave a comment on this PR containing |
|
@corymhall thank you for your response! To be honest, I don't fully understand the exact reasons why this problem occurs, nor do I know how to reproduce it. However, it has been observed that when deploying many stacks containing Lambda functions using the concurrency option in cdk deploy, this issue tends to occur probabilistically. In #23290, an In the moveIntoPlace function related to the problem, the following steps exist:
The
Regarding the
Therefore, in this PR, I am attempting to resolve this by adding a random string to the outputFile passed to the zipDirectory function, as a way to avoid this problem. |
|
@tockn thanks for the detailed explanation! Based on that I think it occurs because a single asset can be used by multiple stacks and each stack will attempt to deploy their own assets in parallel. This means we end up publishing the same asset multiple times (potentially in parallel causing the issue). The team has recently completely reworked the asset publishing mechanism and I've been told that the new mechanism should account for this. The release this week |
I see, I understand now, thank you!
That's great! I'm very much looking forward to it! |
|
I think your analysis is correct, but I don't like the proposed fix. I'd prefer fixing the |
There was still a TOCTOU error in the file asset publishing, which could
lead to `ENOENT: no such file or directory` when assets were being published
in parallel.
The whole `if (fileExists) { delete; }` logic was actually not
necessary, as `fs.rename` will atomically overwrite existing files
already, so we can just call it directly.
Closes #25293.
|
I'm proposing this change instead: #25869 |
There was still a TOCTOU error in the file asset publishing, which could lead to `ENOENT: no such file or directory` when assets were being published in parallel.
The whole `if (fileExists) { delete; }` logic was actually not necessary, as `fs.rename` will atomically overwrite existing files already, so we can just call it directly.
Closes #25293.
----
*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
There are still similar issues of #23290 occurring despite the fixes for #23677 and #24026. In
moveIntoPlaceduring asset building, there is a process to unlink if there is already a file with the same name in the destination directory. However, the check for the existence of this file can sometimes result in a race condition and trigger an ENOENT error.This pull request resolves this issue by adding a random string to the file name of the packaged file itself.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license