cryptomator / dokany-nio-adapter

Dokany-based adapter to provide directory contents specified by a java.nio.file.Path (via dokan-java)
GNU Affero General Public License v3.0
14 stars 4 forks source link

Migrate to Dokany 2.x #52

Open infeo opened 2 years ago

infeo commented 2 years ago

Dokany release the next major version 2.x.

The version includes performance improvements, but is not compatible with the 1.x version.

A migration path is provided in https://github.com/dokan-dev/dokany/wiki/Update-Dokan-1.1.0-application-to-Dokany-2.0.0.

Experiments take place in the branch https://github.com/cryptomator/dokany-nio-adapter/tree/feature/dokan-2.0

infeo commented 2 years ago

After a several experiments, the current state is: Dokany 2.x does not run stable using JNA.

The reasons are unclear, but after a longer (or shorter) time, the running process crashes with NTSTATUS either STATUS_ACCESS_DENIED or STATUS_INTEGER_DIVIDE_BY_ZERO. In rare cases memory dump is created.

The branch contains the rewrite of the library, discarding the type safe enumerations and mainly dealing with "raw" values. The class ReadOnlyAdapter contains a minimal read only filesystem implmentation, to test the stability of the implementation.

Liryna commented 2 years ago

@infeo couple of fixes were introduced since in the library to deal with memory corruption and synchronisation. If you can give another try and share any issues that would be appreciated!

infeo commented 2 years ago

Thanks for the reminder! I gave it another whirl, and it seems that the mount is stable after a certain time has passed (on my machine ~15s).

In the new implementation DokanCreateFileSystem is used. If the calling thread leaves the scope of the surrounding java method before the aforementioned time span passed, it is very likey the application crashes with one of the above error codes. The exception triggering dll is ntdll.dll, so i honestly don't know where the error is located.

@Liryna Is there some critical (background) operation at the mount which can explain that the mount stabilizes after a certain time?

Liryna commented 2 years ago

Are you able to get the library logs in that case ? We could see if there is an unmount happening.

Could it be possible that somehow java is releasing the allocated native memory when going out of scope ? Like the delegates are free and it natively crash.

infeo commented 2 years ago

Could it be possible that somehow java is releasing the allocated native memory when going out of scope ?

It is possible from my naive point of view, but unlikey, because the used libraries are stable and mature. The two possibilites i see are either a wrong memory mapping (e.g. wrong start address) or memory is freed to early.

I have collected logs of a crash including driver output and the event logging: dokan_crashLog.zip

@Liryna Since this is a lot to look through, can you point me in some directions where to look at or what to look for?

infeo commented 2 years ago

Also noteworhty: I can almost certainly trigger the crash, when accessing the volume properties (event without implementing DokenGetVolumeInformation.

Liryna commented 2 years ago

Thanks for the logs @infeo . What I am seeing is that the driver start to unmount due to the keepalive handle being closed happening when windows release the resource after the app crashes.

All the communication looks good and properly received by the kernel.

Quickly looking at https://github.com/cryptomator/dokany-nio-adapter/tree/feature/dokan-2.0 the API change seems to have been applied correctly but it is hard without a "real" diff.

When I pointed at the native resources being released, I was thinking at the mount parameters and especially dokanOperations. The dotnet wrapper was successfully migrated but even if it is also a mature library this element had to be redesigned as it was freed by the GC shortly after the call as it is now async and GC has no clue it is still used natively which produce the same type of crash. https://github.com/dokan-dev/dokan-dotnet/issues/280 I have 0 experience in Java so I cannot say if that's the case but it should be looked at and ensure that's not the case.

infeo commented 2 years ago

this element had to be redesigned as it was freed by the GC shortly after the call as it is now async and GC has no clue it is still used natively which produce the same type of crash.

giphy

My god, that was the reason. Instead of the operations struct, the dokanOptions were locally created and with the next gc run freed.

Thanks a lot, now it runs without crashing. :D

hpvd commented 1 year ago

there have been some new versions of dokany. Together with learnings from https://github.com/cryptomator/dokany-nio-adapter/issues/52#issuecomment-1059291107 -> maybe one should give it a new try. Latest version is 2.0.5.1000 from 2022-07-04 https://github.com/dokan-dev/dokany/blob/master/CHANGELOG.md

The benchmarks of v2 look really promising and should make migration well worth it: https://github.com/dokan-dev/dokany#benchmark-v1511000-vs-v2031000

hpvd commented 1 year ago

related to: https://github.com/cryptomator/cryptomator/issues/2001

hpvd commented 1 year ago

There is a improved doc with latest revision from 7th june 2022 on How to Update Dokan 1.1.0 application to Dokany 2.0.0 https://github.com/dokan-dev/dokany/wiki/Update-Dokan-1.1.0-application-to-Dokany-2.0.0

=> possibly it's worth a look, if it (in latest revision) contains the mission pieces of information...

hpvd commented 1 year ago

another reason for dokany v2 support (beside e.g. speed see https://github.com/dokan-dev/dokany#benchmark-v1511000-vs-v2031000):

....the WINFSP option does not provide case-insensitive DIR and TAB-Completion as DOKANY does.

https://github.com/cryptomator/cryptomator/issues/2001#issuecomment-1274445330