Skip to content
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

Fix shadow copy up to date check on startup (#52831) #55659

Closed
wants to merge 2 commits into from

Conversation

BrennanConroy
Copy link
Member

Backport of #52831

Fix shadow copy up to date check on startup

Description

If using shadow copy in ANCM and you shutdown the app (e.g. app_offline.htm) then delete a subfolder (with a dll), starting up the app can cause a crash that can only be resolved by adding back the deleted folder. Or resolved by deleting the shadow copy folder (either manually or with cleanShadowCopyDirectory setting turned on).

Customer Impact

App permanently crashes on startup until moving app to a new directory, cleaning shadow copy folders, or readding removed file. This is a hard to diagnose issue as it's just a random crash with little info about what happened.

Regression?

  • Yes
  • No

Risk

  • High
  • Medium
  • Low

Code was obviously wrong once the issue was looked into, very simple fix. Added a test to verify.

Verification

  • Automatic

Packaging changes reviewed?

  • Yes
  • No
  • N/A

@BrennanConroy BrennanConroy added the Servicing-consider Shiproom approval is required for the issue label May 10, 2024
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions label May 10, 2024
@dotnet-policy-service dotnet-policy-service bot added this to the 8.0.x milestone May 10, 2024
@@ -231,7 +231,7 @@ bool Environment::CheckUpToDate(const std::wstring& source, const std::filesyste
auto sourceInnerDirectory = std::filesystem::directory_entry(path);
if (sourceInnerDirectory.path() != directoryToIgnore)
Copy link
Member

@halter73 halter73 May 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is unchanged, but why is this logic different than CopyToDirectoryInner? It feels like we should use the same logic in both places if possible. We could do that in main though.

            auto sourceInnerDirectory = path.path();

            if (sourceInnerDirectory.wstring().rfind(directoryToIgnore, 0) != 0)

And what is auto sourceInnerDirectory = std::filesystem::directory_entry(path); doing? Isn't path already a directory_entry?

@BrennanConroy BrennanConroy added Servicing-rejected and removed Servicing-consider Shiproom approval is required for the issue labels May 13, 2024
@BrennanConroy
Copy link
Member Author

Backport to 8.0 rejected. Signal is low for this and there is a workaround listed in the issue and below. If someone hits the issue they can find the stack trace from event viewer in the original github issues.

Can resolve the issue by deleting the shadow copy folder (either manually or with cleanShadowCopyDirectory setting turned on). Can avoid the issue by setting cleanShadowCopyDirectory or by not changing folders/files of published app unless shadow copy directories are removed.

@BrennanConroy BrennanConroy deleted the brecon/bpdirswap branch May 13, 2024 21:10
@dotnet-policy-service dotnet-policy-service bot modified the milestones: 8.0.x, 8.0.6 May 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions Servicing-rejected
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants