dokan-dev / dokan-dotnet

Dokan DotNet Wrapper
http://dokan-dev.github.io
MIT License
462 stars 116 forks source link

Unmanaged struct keep-alive logic for async mount #282

Closed LTRData closed 2 years ago

LTRData commented 2 years ago
Liryna commented 2 years ago

Thanks @LTRData ! Very appreciated that you looked into this so quickly!!!

Liryna commented 2 years ago

This PR looks fine. The test issue comes from the overlapped tests which one of them assert due to unexpected data. dotnet test --filter OverlappedTests Otherwise all other test pass.

Thanks @LTRData for your PR! Merging it and I will try to find out what is happening.

LTRData commented 2 years ago

Thanks! There are some obvious errors in the tests. For example a handle could be closed before I/O is complete in some cases. Correcting that helped a bit but not completely. There is another issue somewhere that I cannot find the root cause for right now. The implemention passes other file system test cases I have tried so I am pretty sure there is nothing actually wrong, it is just the test cases in the solution that have some problems.

Liryna commented 2 years ago

Well the latest CI run just passed https://ci.appveyor.com/project/Liryna/dokan-dotnet/builds/42134826 ... even if locally I still have failures 😶‍🌫️ I have been debugging the kernel to see if all data were completed correctly and it is the case. So Yeah I am on your side that something is wrong with the test but I am not able to point what.

Correcting that helped a bit but not completely

Could you share the change ?

Liryna commented 2 years ago

Oh just found! https://github.com/dokan-dev/dokan-dotnet/commit/4456212fbd536f658cafa346bf478e4b408a0c97 Have you seen any other issues ? It is still flaky for me

LTRData commented 2 years ago

I have not found anything else that looks like it could have any real impact on anything, but the code is a bit difficult to follow. It could probably be worth it to try to write entirely new test cases for overlapped tests. In earlier versions the threading was different in the driver and so I guess it probably happened to work even though something was probably broken with the test cases already.