Giter VIP home page Giter VIP logo

Comments (15)

Liryna avatar Liryna commented on August 30, 2024

Hi @js69 ,

On my side Delete On Close is correctly set otherwise the C# Mirror would never been able to remove files.

Having the flag in Close is useless because in Windows logic, the file has to be delete in the Cleanup.

Have you been able to make a simple test that delete a file ? To see if this only happen with Excel or in every cases ?

Be aware to dokan-dev/dokany#249, is it is the same behaviour that you face, it is probably an issue directly in Dokan and not in the C#.

from dokan-dotnet.

js69 avatar js69 commented on August 30, 2024

The C# mirror does not handle the DeleteOnClose case itself at all - that is done by Windows for the FileStream you create. The same is probably true for the C mirror sample.

The Cleanup never receives a DeleteOnClose=true flag from any Office application, as far as I can tell.

This might be fixable in the driver if it were to remember the FILE_DELETE_ON_CLOSE flag set on Create when it calls Cleanup later. In any case, CleanupProxy needs to have that flag set to allow it to handle the case correctly.

from dokan-dotnet.

Liryna avatar Liryna commented on August 30, 2024

What you describe is already made dokan-dev/dokany@770d2a7#diff-f8956c408fab8dbdc1c6ccfbd0a1035cR108

We save the FILE_DELETE_ON_CLOSE and set it in the Cleanup.

In the C# Mirror logs that you gave, DeleteOnClose is not set during CreateFile.
I see in your logs that there is two CreateFile and one with DeleteOnClose but are you sure that there is no DeleteOnClose flag set ?

DeleteOnClose directly come from dokan.dll and the flag is set here https://github.com/dokan-dev/dokany/blob/7c53ac77393e1430ad228b2e514f1f4a5d7fd247/dokan/dokan.c#L543

BUT there is probably something with Office 2010 and the magic of Windows that we do not handle.
I only think that if you see the request CreateFile with the flag delete, it have to be also set in Cleanup.

from dokan-dotnet.

js69 avatar js69 commented on August 30, 2024

I don't see in the Dokan driver cleanup.c anything that handles the Delete On Close flag.

Shouldn't there be something similar like what was added in create.c?

if (irpSp->Parameters.Create.Options & FILE_DELETE_ON_CLOSE) {
  fcb->Flags |= DOKAN_DELETE_ON_CLOSE;

from dokan-dotnet.

Liryna avatar Liryna commented on August 30, 2024

In the kernel side, we only have the information during the Create. Therefore, we save the flag in fcb->Flags dokan-dev/dokany@770d2a7#diff-86caa6b16e4044982075fe5d5bc59915R514
and we set it back in Cleanup event dokan-dev/dokany@770d2a7#diff-f8956c408fab8dbdc1c6ccfbd0a1035cR108

Or I am missing something in what you describe 😢

from dokan-dotnet.

js69 avatar js69 commented on August 30, 2024

OK, getting back to the issue:
info.DeleteOnClose is never true in the C# mirror Cleanup when opening and closing a file in Excel 2010 or Excel 2016. Could you please trace if that flag is really correctly set in the driver code's Cleanup?

from dokan-dotnet.

Liryna avatar Liryna commented on August 30, 2024

@js69 , I will take a look this weekend !
Just to be sure, does the behaviour also happen with Excel 2016 with C mirror ?

from dokan-dotnet.

Liryna avatar Liryna commented on August 30, 2024

Hi @js69 ,

From my observation (office 2016), the flag is correctly set during the CreateFile BUT during the Cleanup, the flag is not set.
The C and C# mirror succeed to delete the temporary file because they open the flag with DeleteOnClose during the CreateFile.

I agree that Cleanup should send the DeleteOnClose flag but I have no idea why does the fcb->Flags; does not have the correctly value set from CreateFile.

Also the Context is seems to be correctly set so it is strang that flag is not correctly restored.
@marinkobabic Do you have any idea why this would happen in some cases (only have been to reproduce with office) ?

[DokanFS] ==> DokanCreate
[DokanFS]   ProcessId 3864
[DokanFS]   FileName:\liryna\Desktop\~$hey.xlsx
[DokanFS]    IdType = VCB
[DokanFS]   IrpSp->Flags = 0
[DokanFS]   Allocate FCB
[DokanFS]   FILE_DELETE_ON_CLOSE is set so remember for delete in cleanup
[DokanFS] FLAGS VALUE = 16

from dokan-dotnet.

marinkobabic avatar marinkobabic commented on August 30, 2024

It's clear to me what happens:

  1. CreateFile "myfile" + DELETE_ON_CLOSE
  2. CreateFile "myfile + OPEN_EXISTING
  3. CloseFile "myfile"
  4. CloseFile "myfile

If the order is like above, the CleanUp will not set the flag DeleteOnClose. Here the code causing the problem

https://github.com/dokan-dev/dokany/blob/master/sys/create.c#L591-L597
It's the https://github.com/dokan-dev/dokany/blob/master/sys/create.c#L596 which removes the flag during the second create. To fix this bug, the else line should be removed.

So but now you will get the flag during both CleanUp which may be wrong (example above 3,4). So after this line here https://github.com/dokan-dev/dokany/blob/master/sys/cleanup.c#L106 we need to check if the fcb->FileCount > 1 and if the flag DOKAN_DELETE_ON_CLOSE is set. If this is true, the flag should be removed from eventContext->FileFlags.

Hope you agree 😄

from dokan-dotnet.

Liryna avatar Liryna commented on August 30, 2024

🎉 Yeah @marinkobabic , it is exactly what is happening! that's sounds like a good fix 😃

But is it not possible to have a personnalized flag for each request ? because with your fix, it is always the last cleanup that get the delete on close.

from dokan-dotnet.

marinkobabic avatar marinkobabic commented on August 30, 2024

@Liryna

This makes also sense. Only if there is no handle on the file, the file should be removed.

Thanks

Marinko

Von: Liryna [mailto:[email protected]]
Gesendet: Montag, 20. Juni 2016 12:57
An: dokan-dev/dokan-dotnet [email protected]
Cc: Babic Marinko [email protected]; Mention [email protected]
Betreff: Re: [dokan-dev/dokan-dotnet] DeleteOnClose flag (#83)

🎉 Yeah @marinkobabic https://github.com/marinkobabic , it is exactly what is happening! that's sounds like a good fix 😃

But is it not possible to have a personnalized flag for each request ? because with your fix, it is always the last cleanup that get the delete on close.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub #83 (comment) , or mute the thread https://github.com/notifications/unsubscribe/ADHSOMl-U1CJLnCQ6c35LhM84PrFv3v9ks5qNnHmgaJpZM4I0J_L . https://github.com/notifications/beacon/ADHSOPNykrd2ul33ROCvwlVPhOTnMdCYks5qNnHmgaJpZM4I0J_L.gif

from dokan-dotnet.

marinkobabic avatar marinkobabic commented on August 30, 2024

When the information is on CCB then it should be a personalized information. The question is when does windows delete the file?

CreateFile
CreateFile + DELETE_ON_CLOSE
CloseFile above

Wait 30 Seconds
CloseFile using the handle of the first create.

If the file is closed after the wait, the the fix is fine. If the file is removed before the wait, then we should put the flag on CCB.

from dokan-dotnet.

Liryna avatar Liryna commented on August 30, 2024

I made some test in current status of dokan:

(1) CreateFile
(2) CreateFile + DELETE_ON_CLOSE
(2) CloseFile - Delete flag // DeleteFile ERROR_ACCESS_DENIED
(1) CloseFile - Delete flag
(1) CreateFile + DELETE_ON_CLOSE
(2) CreateFile
(2) CloseFile - No Delete flag
(1) CloseFile - No Delete flag

With this commit:dokan-dev/dokany@f57b054

(1) CreateFile
(2) CreateFile + DELETE_ON_CLOSE
(2) CloseFile - Delete flag // DeleteFile ERROR_ACCESS_DENIED
(1) CloseFile
(1) CreateFile + DELETE_ON_CLOSE
(2) CreateFile
(2) CloseFile
(1) CloseFile - Delete flag

So your idea @marinkobabic to put on CCB since to be the good one ! 👍

Before adding it to master branch, I just would like to know if https://github.com/dokan-dev/dokany/blob/master/sys/create.c#L596 is really needed and what is the purpose of it ? (it seems like the flag on fcb is used for DeletePending)

from dokan-dotnet.

marinkobabic avatar marinkobabic commented on August 30, 2024

Hi Liryna

The line makes no sense, so you can remove it. Good job!

Thanks
Marinko

Von Samsung Mobile gesendet

from dokan-dotnet.

Liryna avatar Liryna commented on August 30, 2024

Ok great !

Thank you for you expertise @marinkobabic 😃 and @js69 for your report ! It was hard to see :)

Fixed on master with dokan-dev/dokany@59d08ff

from dokan-dotnet.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.