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

Call for better Mirror example for application to manage file context #1043

Closed
kice opened this issue Jan 9, 2022 · 5 comments
Closed

Call for better Mirror example for application to manage file context #1043

kice opened this issue Jan 9, 2022 · 5 comments

Comments

@kice
Copy link

kice commented Jan 9, 2022

Looks like all operations have requests FileName as parameter. However it suggests that we could store application specific information to DOKAN_FILE_INFO.Context.

I checked some issues and looked at the PDOKAN_OPERATIONS Struct Reference, but it looks like it does not say we have to reopen for ReadFile and WriteFile. Only thing I found useful regarding this was here:

https://github.com/dokan-dev/dokan-dotnet/blob/f30f724babf864863700293b2bcb2edef06bc365/sample/DokanNetMirror/Mirror.cs#L282

So only ReadFile/WriteFile were affected, other operations will be fine for memory mapped file operations? If so, please add this kind of information to the documentation.

Why not add an optional flush callback?

@Liryna
Copy link
Member

Liryna commented Jan 9, 2022

Hi @kice ,

This case is documented here https://dokan-dev.github.io/dokany-doc/html/ and handled by samples.

Maybe we could document it directly in the callback documentation. Would you be willing to open a pull request for this ?

Fyi there is another issue open for this case #1016

@kice
Copy link
Author

kice commented Jan 9, 2022

Btw, I just noticed that it will close the handle after re-open the file object. Why not keep the new handle back to context?

if (opened)
CloseHandle(handle);
return STATUS_SUCCESS;

After a file object was successfully created, will dokan call back PDOKAN_OPERATIONS.Cleanup only once during whole filetime? Or just to avoid some magic behavior from the old source code?

I would like to see store a struct in mirror DOKAN_FILE_INFO.Context, like storing not only the file handle, but also the real file path and a operations counter to app file context.
This should give a very good example to life time management and thread-safety.

@kice kice changed the title Why reopen file for ReadFile/WriteFile if Handle is invalid? Call for better Mirror example for application to manage file context Jan 9, 2022
@Liryna
Copy link
Member

Liryna commented Jan 9, 2022

Exact PDOKAN_OPERATIONS.Cleanup will only be called once by the Windows Kernel.

The mirror sample is only storing the HANDLE. It has no use for a struct and the life time of both would be handled the same.

Doc was improved 6bbbbc7

@kice
Copy link
Author

kice commented Jan 9, 2022

So far I what understand for mirror if using sturct

  1. ZwCreateFile: allocate memory for context, store handle
  2. Some operations
  3. maybe Cleanup if it is Memory Mapped File: close handle, DO NOT free the memory
  4. ReadFile/WriteFile/GetFileInformation. If Cleanup was called, we need to reopen file and make new Context, and MUST release the reopen object before return (commit change to dirty data)
  5. Cleanup: close handle, DO NOT free the memory
  6. CloseFile: release memory

For mem mapped file, we should always return the last-written-to-disk data to read operation, and any write operation must be committed to actual file; for read and write, more or less like we have transferred the internal file object to the kernel as soon as use request to map the file object.

Can we say this way, for mem mapped file, we are also dealing with "coherence", and mirror example choose to ignore the dirty data in the memory, which is the correct way to do?

Some other thought as well, if Cleanup is called upfront, I may easily assume that we are committing the final dirty data to the actual file, and it should be the last operation for that file object. However I does not understand why read would be the last operation, or why Cleanup would call before read (async operation?).

And may I suggest that it is a good idea that check DokanFileInfo->PagingIo before reopen the file object in ReadFile/WriteFile/GetFileInformation, to make sure it is mmap.

@Liryna
Copy link
Member

Liryna commented Jan 9, 2022

This looks to be exact and follow the documentation. Let me know if you find any issue in the project and I will be glad to look into it.

@Liryna Liryna closed this as completed Jan 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants