Prevent garbage collection of delegates during DokanMain

[uholzer]

This is a weird one: When calling Dokan.Mount, my application reproducibly behaves as follows:

1. It makes several successful calls to my implementaion of IDokanOperations.
2. After a short time, one such call fails, with the callbackOnCollectedDelegate MDA1 reporting that the delegate DokanNet!DokanNet.DokanOperationProxy+GetFileInformationDelegate::Invoke was called from unmanaged code even though it had already been garbage collected. This happens while the call to Dokan.Mount is still running.

My take on this is as follows:

• Dokan.Mount calls the unmanaged DokanMain of the Dokany C library, passing an instance of the class DOKAN_OPERATIONS, which requires marshalling3.
• DOKAN_OPERATIONS is considered unblittable4 because it contains delegates. Therefore, the marshaller creates a copy5 of this class in the correct format and passes a pointer to that copy to DokanMain.
• As soon as a copy is created, the original DOKAN_OPERATIONS is not strictly required anymore and can be garbage collected.
• Garbage collection may happen right before to call to DokanMain. Apparently, it may also happen while DokanMain is running due to multi-threading.
• The delegates are garbage collected too, since they are only referenced by DOKAN_OPERATIONS.

This pull request is my attempt at fixing it. It has solved the problem for me. However, I am not quite sure if this works out in all cases. The garbage collector can basically do two things:

• Remove data from memory. In my pull request, this is prevented using GC.KeepAlive2.
• Relocate data (compaction). Could this happen to delegates and is it prevented? Would they need to be pinned5 in some way?

[LTRData]

I am sorry but I do not think your analyze of what is causing this problem could be correct. Objects involved in p/invoke calls and objects they are referring to should stay alive during the p/invoke call and that includes the original managed object that the temporary unmanaged structure was created from. Also, I would have seen this problem a lot in my tests if a problem like this was showing up here. I assume that there might be something else going on that is worth investigating though. Could you share more code where this happens?

[uholzer]

On the current master branch it happens to me as follows:

• I run the DokanNetMirror sample with the Release configuration in VisualStudio's debugger. I browse in the file system and everything works fine.
• I run the RegistryFS (net46) sample with the Debug configuration in the debugger. I browse in the file system, which also seems to work correctly.
• I run the RegistryFS (net46) sample with the Release configuration in the debugger. It starts up properly but after browsing for a short amount of time in the file system, listing a few folders at most, I get the "CallbackOnCollectedDelegate".

This is with Visual Studio 2019 and .NET Framework 4.8 installed if I am not mistaken.

[LTRData]

On the current master branch it happens to me as follows:

• I run the RegistryFS (net46) sample with the Release configuration in the debugger. It starts up properly but after browsing for a short amount of time in the file system, listing a few folders at most, I get the "CallbackOnCollectedDelegate".

Okay thanks. I'll take a look and see what I can find there!

[LTRData]

Ah, yes I see now. There is some strange optimization in release build that causes this. It is possible to avoid with GC.KeepAlive but not in the way you have done it in the pull request. A GC.KeepAlive call needs to be after the last unmanaged call where the object needs to stay alive. (You were calling it immediately after object creation which has no effect.) I made a new pull request with just two calls to GC.KeepAlive after the DokanMain call.

[uholzer]

I do believe that my version is correct because the call to GC.KeepAlive comes after DokanMain. However, your pull request works for me too.

[LTRData]

I do believe that my version is correct because the call to GC.KeepAlive comes after DokanMain. However, your pull request works for me too.

Yes you are right, sorry about that. I misread the diff in some strange way. My bad!

[LTRData]

...and for the record, this is not at all expected behaviour or a documented requirement. This is most likely a bug either in the optimizing parts of the compiler or, more likely in this case, a bug in .NET Framework. I cannot seem to reproduce it in .NET Core for example and with small seemingly unrelated changes somewhere it no longer happens in .NET Framework either. So in my opinion we should really consider this a workaround for a framework (compiler, runtime etc) bug, not a bugfix for DokanNet itself.

[Liryna]

Thanks to both of you to look into this issue! I would say if LTRData small PR do the job, we should use it. Uholzer, in your PR you have introduced the Configuration instance function which is a good idea, could you make a new pull request or reused the existing one just for that change ?

@LTRData your PR has a bunch of previous changes. Could you clean the history so I can merge a single commit ?

[LTRData]

@LTRData your PR has a bunch of previous changes. Could you clean the history so I can merge a single commit ?

Yes you are right, it has become a bit messy since last things I did here. I'll back up a couple of steps and make a clean simple commit instead.

[Liryna]

I made a new release with you guys fix https://github.com/dokan-dev/dokan-dotnet/releases/tag/v2.0.1.1

Thanks again! 😎 🏆