<filesystem>: remove does not delete read-only files

[SibiSiddharthan]

Descibe the bug
std::filesystem::remove() does not remove read-only files
The issue with this is that STL sets the FILE_ATTRIBUTE_READONLY bit when we remove the write permissions for a file.
remove should remove such files also.

Command-line test case

C:\Temp>type repro.cpp
#include <filesystem>
#include <iostream>
#include <fstream>

using namespace std;
namespace fs = std::filesystem;

int main()
{
	fs::path testfile("testfile");

	// Create the file
	fstream file;
	file.open(testfile, ios::out);
	file.close();

	// Remove write permissions
	fs::permissions(testfile,
				fs::perms::owner_write | fs::perms::group_write | fs::perms::others_write,
				fs::perm_options::remove);

	error_code E;
	//POSIX remove should remove this as well.
	fs::remove(testfile, E);
	//Should print '5 Access is denied.'
	cout << E.value() << " " << E.message() << endl;
}


C:\Temp>cl /EHsc /std:c++17 /W4 /WX .\repro.cpp
Microsoft (R) C/C++ Optimizing Compiler Version 19.23.28019.1 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

repro.cpp
Microsoft (R) Incremental Linker Version 14.23.28019.1
Copyright (C) Microsoft Corporation.  All rights reserved.

/out:repro.exe
repro.obj

C:\Temp>.\repro.exe
5 Access is denied.

Expected behavior
std::filesystem::remove() should remove 'testfile'

STL version
Versions that support std::filesystem

[CaseyCarter]

Bug confirmed. I assume we could fix this by clearing the read-only attribute and retrying when access is denied, but hopefully there's a more elegant way.

[SibiSiddharthan]

I think we should try and clear the read-only attribute before setting FILE_FLAG_DELETE_ON_CLOSE. In this way we avoid retrying for the failure case.

[CaseyCarter]

I think we should try and clear the read-only attribute before setting FILE_FLAG_DELETE_ON_CLOSE. In this way we avoid retrying for the failure case.

I expect remove is used at least two orders of magnitude more frequently on non-readonly files, in which case it would be better on average to avoid extra work for the non-readonly case at the expense of the readonly case.

[SibiSiddharthan]

If I understand correctly, the file is being deleted when the destructor for _Fs_file is being called at the end of __std_fs_remove function. Then to retry in case of failure, the changes have to done to the 2 overloads of remove in the filesystem header file. Is this correct/acceptable?

[egorpugin]

This is not completely fixed.
The issue especially happens on .git folder after some time of using that repo.
fs::remove() returns true for some .git/objects/anynumber/anyfile, but further check in fs::remove_all() finds that not all files were removed.

[StephanTLavavej]

Such files are indeed readonly, so I'll reactivate this bug:

C:\GitHub\STL\.git\objects\f3>attrib
A    R               C:\GitHub\STL\.git\objects\f3\28589c3326fa4f97566f1e1c3386ebe05f14d8
A    R               C:\GitHub\STL\.git\objects\f3\44ea8a1d4994f2a27733410a671d36b1916817
A    R               C:\GitHub\STL\.git\objects\f3\5d7661c0c87a71a152ed0b09ea69fa8efc1d67
A    R               C:\GitHub\STL\.git\objects\f3\8d2f7ad8debd6b0d75257b538a40f09b77122d
A    R               C:\GitHub\STL\.git\objects\f3\df360f3a4b8220fc897b5faac38b54dee537bd

[SibiSiddharthan]

Hey, I am trying to debug this. But the objects inside the git folder are getting deleted with fs::remove. I am not sure how some objects aren't getting deleted. Maybe some external process is holding file contents in memory relinking the files.