Creating a file after deleting it may fail

[zqlustc]

Feature request can skip this form. Bug report must complete it. Check List must be 100% match or it will be automatically closed without further discussion. Please remove this line.

Environment

• Windows version:
• Processor architecture:
• Dokany version:
• Library type (Dokany/FUSE):

Check List

• I checked my issue doesn't exist yet
• My issue is valid with mirror default sample and not specific to my user-mode driver implementation
• I can always reproduce the issue with the provided description below.
• I have updated Dokany to the latest version and have reboot my computer after.
• I tested one of the last snapshot from appveyor CI

Description

I ues the PickerHost.exe to edit the picture and save as A31.jpg(It is located in dokan mount disk). It probabilistic failed. ProcessMonitor trace as show:

So I use the mirror to test. After I delete a file(CreateFile -> DeleteFile, there is no CloseFile), I create the file immediately. It will fail in mirrorfs.I find it failed here through the dokan1.sys driver debug.

dokany/sys/create.c

Lines 691 to 693 in 685aab7

	if (fcb->FileCount > 1 && disposition == FILE_CREATE) {
	status = STATUS_OBJECT_NAME_COLLISION;
	__leave;

but it will success in NTFS. Could we support it as same as NTFS? Thanks.@Liryna

Logs

Please attach in separate files: mirror output, library logs and kernel logs.
In case of BSOD, please attach minidump or dump analyze output.

[Liryna]

@zqlustc Hi,

Thank you for the report.

Could you provide a small sample code or manual steps to reproduce the issue ?

[zqlustc]

A small sample code:

int CreateAfterDelete(string sPath)
{
    cout << "openAndDeleteTest start" << endl;

    OFSTRUCT _buffer;
    HANDLE hFILE;
    HANDLE hcFILE;

    cout << sPath << endl;
    hFILE = CreateFileA(sPath.c_str(), GENERIC_READ | GENERIC_WRITE, FILE_SHARE_DELETE | FILE_SHARE_READ | FILE_SHARE_WRITE, NULL, OPEN_EXISTING, FILE_ATTRIBUTE_NORMAL, NULL);
    if (hFILE == INVALID_HANDLE_VALUE) {
        cout << "ERROR: No " << sPath << "file" << endl;
        return 0;
    }

    int i = 0;
    bool ret = DeleteFileA(sPath.c_str());
    if (ret == false)
    {
        cout << "ERROR: delete failed.\n";
        return 0;
    } else {
        cout << "SUCCESS: file have been deleted\n";
    }

    Sleep(5000);

    hcFILE = CreateFileA(sPath.c_str(),
        GENERIC_WRITE | GENERIC_READ,
        0,
        NULL,
        CREATE_NEW,
        FILE_ATTRIBUTE_NORMAL,     
        NULL);
    if (hcFILE == NULL) {
        cout << "ERROR: new create failed \n";
        return 0;
    } else {
        cout << "SUCCESS: new file create success\n";
    }
    CloseHandle(hFILE);
    return 0;
}

It can excuted success in the NTFS disk folder, After the file is deleted, the file was created in 5 seconds. But it failed in mirrorfs folder

[zqlustc]

Can you reproduce the issue? Please @me if you meet any problem, thanks @Liryna

[Liryna]

@zqlustc I will not have much to look into this in the next days sorry.
My guess is that maybe the counter used is not decrement in the correct irp. Have to check the fastfat source code and compare.

[zqlustc]

@zqlustc I will not have much to look into this in the next days sorry.
My guess is that maybe the counter used is not decrement in the correct irp. Have to check the fastfat source code and compare.

OK, if you have any progress ，please let me know. Thanks very much!

[zqlustc]

Hello, this problem still pester me, could you reproduce it by the POC and have a look, thanks very much?@Liryna

NTFS could create file when the file requested successfully opened already existed. But we do not know the file is or not existed in dokan drivers here.

dokany/sys/create.c

Lines 691 to 693 in 685aab7

	if (fcb->FileCount > 1 && disposition == FILE_CREATE) {
	status = STATUS_OBJECT_NAME_COLLISION;
	__leave;

In other word, IF the file is not existed, we should allow apps create the file.

[the-Arioch]

Hello @zqlustc

You did not posted the (whole, textual) log from ProcMon, and only a picture of small part of the log.
However, what caught my eye was - there is no a single delete operation in the picture.
You say you was deleting the file, but your picture clearly says you did not.

You mention the DeleteFileA function and here is what Microsoft say about it:

The DeleteFile function fails if an application attempts to delete a file that has other handles open for normal I/O or as a memory-mapped file (FILE_SHARE_DELETE must have been specified when other handles were opened).

The DeleteFile function marks a file for deletion on close. Therefore, the file deletion does not occur until the last handle to the file is closed. Subsequent calls to CreateFile to open the file fail with ERROR_ACCESS_DENIED.

https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-deletefilea

IOW your description of an application (CreateFile -> DeleteFile, there is no CloseFile) is weird and it is allegedly doing what it should not have do.
You can not legally both delete a file and have an open handle to it. You can, with some specific tricks, make DeleteFile call not fail immediately, but even then it does NOT delete the file but only schedules it to be deleted somewhen later.

I believe you better to re-think WHAT your application is trying to achive and to do it in some legit way.
Or, if you try to take advantage of some Windows/NTFS bug, then Dokany seems to have no part in it.

[Liryna]

@the-Arioch Here SetDispositionInformationFile is used to mark the file for deletion and the handle looks to be closed right after.
The latest CreateFile report a file not found and then a name collision.
There must be an issue with the before-last CreateFile that has kept a trace of a possible file open which incorrectly failed the last one.

[the-Arioch]

SetDispositionInformationFile is used to mark the file for deletion

oooops, my lapse! I was assuming even offset deletion would still yeild a procmon-detectable deletefile call. Did not look into params. My bad.

and the handle looks to be closed right after

And that contradicts to the workflow zqlustc outlines. So, frankly, i expect race conditions or something in his app, where the handle is eventually closed by SOME worker in indeterministic async way.

I'll see if i be really fired up in October to write his sample code in Pascal ( i really hate how C-like code looks, sorry ) and make a try on real NTFS. I do not expect that to be actually working, according to my reading of MSDN.

MSDN somehow draws a line between DELETE and RENAME rights, and it is a known distinction between FAT and NTFS that you can (usually) rename EXE and DLLs of running application on NTFS but not FAT.

I wonder if NTFS to an extent follows UNIX semantics that filename is just a shortcut, a hardlink to file (a void* pointer), and file is anonymous byte storage somewhere (output of malloc, that can be remmbered or lost). NT is actually an VMS offspring not UNIx one, and i next to never met VMS. For DOS and Windows filename and file was one and the same object. For NTFS i am not sure, especialyl that later NTFS versions even introduced symlinks...

BTW, this might be a way out, to try zqlustc 's sample on other filesystems - FAT, UDF, exFAT. Would it universally work there? If not, is there an obligation upon Dokany to implement it? NTFS has many features like alt-streams, ext-attrs, junction points (folder hardlinks) and per-file compression. And i can use them in my application - but then i concisely make that my app bound to NTFS and do not have rights to demand other FSes become NTFS clones.

[the-Arioch]

BTW, do you by chance instantly remember if Dokany implements another auto-delete-on-close semantics, the FILE_FLAG_DELETE_ON_CLOSE flag of CreateFile ?
https://docs.microsoft.com/en-us/windows/win32/api/fileapi/nf-fileapi-createfilea#parameters

I speculate, IF there actually is missynchronization of reference counting in Dokany, then i would expect it to also surface there too.
And yet again maybe be related to cleanup issue in 1016 bug

[Liryna]

I haven't tested it but I think I do remember something like NTFS does allow you to (re)create a file between DeleteFileA and CloseHandle despite what is said in the documentation (again, I could just be totally wrong).
This looks like and edge case that we do not support if that correct. It would be nice to find if the behavior is the same on NTFS and FAT. Then see how FASTFAT code handle this.

[the-Arioch]

That being said, mirror'ed disks are reported as NTFS drives.

I said above

And i can use them in my application - but then i concisely make that my app bound to NTFS

But then, if some disk reports being NTFS and does not act like true NTFS - what should such an app do?

Sorry, i feel tired and have a good night, i am out for today.

Just few words... that "baskslah issue" would mean hardening (in defensive programming sense) your driver which is not easy, and especially not easy as you seem to not having Dokan driver API documentation even for yourselves.

Idea that dokanlets should have their own copy of dokan1.dll also leads to that, you have to have a stable driver API even if DLL API would be changing. and that mean driver API should be documented.

Drankly, i'd rather have it the opposite way, stabilze DLL API and had freedom to change driver-to-DLL interface as often as i see fit.

[Liryna]

@the-Arioch There is one driver installer for X apps with possible diff versions of the library. You cannot have the freedom to change the drive-to-DLL interface as you want.
Our logic is to change the major api version when we break this compatibility but we try not to.

you seem to not having Dokan driver API documentation even for yourselves

Do not forget that this is open source. Anyone can send patch or open an issue. I am always open to review anything and give help.
What I want to say that there is no "you" but "we".

[the-Arioch]

Sorry, seems I am afraid i feel sick so sorry if I am not coherent. But i believe when you make some API stable and rely upon it it has to be documented. Maybe not publicly but at least among devs. In case you have a bug, you often find two parts of your system talking different languages, but which is in the wrong, which one to be fixed? Without some consensus fixed in writing it becomes hard to figure it out. Yes, I know how cursed all calls to write the doc sound, I know...

[Liryna]

@the-Arioch Exact, it is important to have doc and we should all write it.
I have contributed most of the library documentation. Maybe on day I will take the time to write of for the driver. Or maybe someone will open a pull request and contribute. You never know !

[zqlustc]

Hi, @Liryna, @the-Arioch
I have test the FAT using the sample code, it is different with NTFS.
(1) Between the DeleteFileA and CreateFileA, the file view is in the file explore, users could see the file
(2) CreateFileA operation return success, and the file was deleted after CloseHandle. There is no a new file existed after sample code over.

[zqlustc]

@the-Arioch Exact, it is important to have doc and we should all write it.
I have contributed most of the library documentation. Maybe on day I will take the time to write of for the driver. Or maybe someone will open a pull request and contribute. You never know !

You can build a frame and we can fill in the content of each chapter. I am also happy to do something like this