Skip to content
This repository has been archived by the owner on Mar 14, 2024. It is now read-only.

Migrate to Dokany 2.x #52

Open
infeo opened this issue Jan 17, 2022 · 12 comments
Open

Migrate to Dokany 2.x #52

infeo opened this issue Jan 17, 2022 · 12 comments
Assignees
Labels
type:enhancement New feature or request

Comments

@infeo
Copy link
Member

infeo commented Jan 17, 2022

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 infeo added the type:enhancement New feature or request label Jan 17, 2022
@infeo
Copy link
Member Author

infeo commented Jan 17, 2022

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
Copy link

Liryna commented Feb 22, 2022

@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
Copy link
Member Author

infeo commented Feb 24, 2022

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
Copy link

Liryna commented Feb 24, 2022

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
Copy link
Member Author

infeo commented Mar 4, 2022

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
Copy link
Member Author

infeo commented Mar 4, 2022

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

@Liryna
Copy link

Liryna commented Mar 4, 2022

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.
dokan-dev/dokan-dotnet#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
Copy link
Member Author

infeo commented Mar 4, 2022

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

@infeo infeo self-assigned this Mar 5, 2022
@hpvd
Copy link

hpvd commented Sep 7, 2022

there have been some new versions of dokany. Together with learnings from #52 (comment) -> 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
Copy link

hpvd commented Sep 7, 2022

related to:
cryptomator/cryptomator#2001

@hpvd
Copy link

hpvd commented Oct 11, 2022

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
Copy link

hpvd commented Oct 11, 2022

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.

cryptomator/cryptomator#2001 (comment)

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type:enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants