IIS Shadow Copy - bug - The last numbered folder inside the Shadow Copy Directory is reused and keeps old artifacts

[Vasilievski]

Hi,

I got many issues with shadow copy lately, mainly with old dlls not being deleted in the shadow copy folder and decided to analyse the source code.

I found this code chunk:

aspnetcore/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/applicationinfo.cpp

Lines 257 to 337 in 1eda538

	/* Overview of shadow copy logic when enabled. See https://github.com/dotnet/aspnetcore/pull/28357 for more context
	* On first request, ANCM goes through its startup sequence, starting dotnet and sending the request into managed code. During this sequence,
	* ANCM will copy the contents of the app directory to another directory which is user specified. The path to this directory can be absolute or relative.
	* Logs and log files will be written to the app directory rather than the shadow copy directory. app_offline will also only be watched in the app directory.
	* The current directory will be set to the app directory as well as the AppContext.BaseDirectory.
	* On publish of new content to the app directory, ANCM will start debouncing file change notifications for dlls, waiting for a steady state.
	* This is done by resetting a timer each time a dll is changed, eventually triggering the timer once there are no dll changes. Afterwards, shutdown is started,
	* causing the process to recycle.
	* Subfolders are created under the user specified shadowCopyDirectory, where the highest int value directory name will be used each time.
	* It will start at subdirectory with name '0' and increment from there. On shutdown, because dlls are still locked by the running process,
	* we need to copy dlls to a different directory than what is currently running in the app. So in the case where the directory name is '0',
	* we will create a directory name '1' and write the contents there. Then on app start, it will pick the directory name '1' as it's the highest value.
	* Other directories in the shadow copy directory will be cleaned up as well. Following the example, after '1' has been selected as the directory to use,
	* we will start a thread that deletes all other folders in that directory.
	*/
	std::filesystem::path
	APPLICATION_INFO::HandleShadowCopy(const ShimOptions& options, IHttpContext& pHttpContext)
	{
	std::filesystem::path shadowCopyPath;
	// Only support shadow copying for IIS.
	if (options.QueryShadowCopyEnabled() && !m_pServer.IsCommandLineLaunch())
	{
	shadowCopyPath = options.QueryShadowCopyDirectory();
	std::wstring physicalPath = pHttpContext.GetApplication()->GetApplicationPhysicalPath();
	// Make shadow copy path absolute.
	if (!shadowCopyPath.is_absolute())
	{
	shadowCopyPath = std::filesystem::absolute(std::filesystem::path(physicalPath) / shadowCopyPath);
	}
	// The shadow copy directory itself isn't copied to directly.
	// Instead subdirectories with numerically increasing names are created.
	// This is because on shutdown, the app itself will still have all dlls loaded,
	// meaning we can't copy to the same subdirectory. Therefore, on shutdown,
	// we create a directory that is one larger than the previous largest directory number.
	auto directoryName = 0;
	std::string directoryNameStr = "0";
	auto shadowCopyBaseDirectory = std::filesystem::directory_entry(shadowCopyPath);
	if (!shadowCopyBaseDirectory.exists())
	{
	CreateDirectory(shadowCopyBaseDirectory.path().wstring().c_str(), nullptr);
	}
	for (auto& entry : std::filesystem::directory_iterator(shadowCopyPath))
	{
	if (entry.is_directory())
	{
	try
	{
	auto tempDirName = entry.path().filename().string();
	int intFileName = std::stoi(tempDirName);
	if (intFileName > directoryName)
	{
	directoryName = intFileName;
	directoryNameStr = tempDirName;
	}
	}
	catch (...)
	{
	OBSERVE_CAUGHT_EXCEPTION();
	// Ignore any folders that can't be converted to an int.
	}
	}
	}
	int copiedFileCount = 0;
	shadowCopyPath = shadowCopyPath / directoryNameStr;
	LOG_INFOF(L"Copying to shadow copy directory %ls.", shadowCopyPath.c_str());
	// Avoid using canonical for shadowCopyBaseDirectory
	// It could expand to a network drive, or an expanded link folder path
	// We already made it an absolute path relative to the physicalPath above
	HRESULT hr = Environment::CopyToDirectory(physicalPath, shadowCopyPath, options.QueryCleanShadowCopyDirectory(), shadowCopyBaseDirectory.path(), copiedFileCount);

It seems like it looks for the last existing numbered folder, but instead of incrementing the int value to create a new folder, it reuses the int value of the found folder.

I don't quite grasp the difference of responsibility between both those files :
https://github.com/dotnet/aspnetcore/blob/1eda5387220d8c1ee5f3be433d867cae817df4ed/src/Servers/IIS/AspNetCoreModuleV2/AspNetCore/applicationinfo.cpp
and
https://github.com/dotnet/aspnetcore/blob/main/src/Servers/IIS/AspNetCoreModuleV2/RequestHandlerLib/filewatcher.cpp

but in the later, where the shadow copy logic is replicated, the increment is done as expected :

aspnetcore/src/Servers/IIS/AspNetCoreModuleV2/RequestHandlerLib/filewatcher.cpp

Line 397 in 1eda538

directoryNameInt++;

Can you explain the difference between both the logics, and is what I spotted actually a bug ?