dahall / Vanara

A set of .NET libraries for Windows implementing PInvoke calls to many native Windows APIs with supporting wrappers.
MIT License
1.8k stars 195 forks source link

Problem using IExplorerBrowserEvents and SafeHandles: SafeHandles cannot be marshaled from unmanaged to managed #28

Closed tajbender closed 5 years ago

tajbender commented 5 years ago

Hello mates,

currently I'm converting my WindowsExplorer-Replacement to use Vanara 2.1.0 wherever possible. So I implemented a Wrapper for ExplorerBrowser as seen e.g. in the Windows-API-Code-Pack.

What code is involved To get the ExplorerBrowser to work, I had to implement a class defining the following interfaces:

Expected behavior Using IExplorerBrowserEvents, my code should get noticed before trying to browse to a folder.

Describe the bug However, when trying to do so I get the following runtime exception:

Managed Debugging Assistant 'InvalidMemberDeclaration' Message=Managed Debugging Assistant 'InvalidMemberDeclaration' : 'The following error occurred while determining how to marshal the parameters of member 'OnNavigationPending' of type 'IExplorerBrowserEvents': System.Runtime.InteropServices.MarshalDirectiveException: Cannot marshal 'parameter #1': SafeHandles cannot be marshaled from unmanaged to managed. This is most likely due to an incompatible MarshalAs attribute on one of the parameters. '

Final Thoughts Since I'm new to Vanara, I really don't get the point where to fix this. Sorry if I'm wrong, but my assumption is, that since public void OnNavigationPending(Shell32.PIDL pidlFolder) { System.Windows.Forms.MessageBox.Show("OnNavigationPending!"); } is called by Window Explorer's ExplorerBrowser-Implementation, the Marshaller can't marshal the native PIDL-Pointer back to Vanara's safe Implementation of a PIDL.

Any ideas on this would be very appreciated! Thanks in advance, tajbender

dahall commented 5 years ago

Well, this is one of those "Ah, crap" moments. I incorrectly used a SafeHandle to simplify the passing/retrieval of parameters and forgot that .NET doesn't marshal those for in/out COM method parameters. Sorry. Let me look at what I can do to fix it.

tajbender commented 5 years ago

Thanks for your quick reply! I can make a commit to my repository, if you need the actual code state to see what happens...

dahall commented 5 years ago

I don't need your code for now. I think I know what I did wrong. Let me correct that and then send you an assembly to test.

dahall commented 5 years ago

What version of .NET is your project using OR do you have a local NuGet repository you can use for testing? I have made some changes to IExplorerBrowserEvents and ICommDlgBrowser[1-3] that follow the rules for implementable COM interfaces that I'd like for you to try. I've left in the references to PIDL as I think they should work with the changes. If not, then I'll punt and just make those parameter types IntPtr.

tajbender commented 5 years ago

Hi, David, I'm using .NET 4.7.2. I've replaced the NuGet-Package references with local builds of your current Repository, and yes, the exception is gone. (Now I get a access violation, but I dont't think it has to do with this issue, since the code is still in development; I'll care of that tomorrow).

Reverting those IExplorerBrowserEvents parameters to IntPtr finally did the trick.

However, since there have been many changes to Vanara just for this issue, I really hope no one else's code base gets affected through this?!?

A zillion thanks for helping me out so fast! I wish you a happy new year 🥇

dahall commented 5 years ago

Good to hear. These classes/interfaces have not been tested and my guess is that nobody has used them either or they'd be seeing your same problems. I'm working on an update (2.1.2) with these fixes and some additions. I've built a small test app to make sure they work before releasing them this time. Give me a few days. I'll commit as I go so you can continue your work with local builds. Thanks for your help.

tajbender commented 5 years ago

Thanks again, but don't mind, I'm not in a rush, so you have all the time for your next Release. My long-range plan is to drop all my self-implemented P/Invoke stuff with Vanara; since implementing such Win32-Shell-API stuff never has been easy in the last two decades, some few weeks more or less don't make such a great difference for me personally :)

But perhaps I can help you out in the future with one or two bites as soon I'm more familiar with the concepts you're using. As soon as I've news on my side, I'll let you know. So far thank again you very much!

tajbender commented 5 years ago

One last word: Be warned by the following comment in WindowsAPICodePack...ExplorerBrowser.cs:

// The below lines are commented out to decline requests for the ICommDlgBrowser2 interface. // This interface is incorrectly marshaled back to unmanaged, and causes an exception. // There is a bug for this, I have not figured the underlying cause. // Remove this comment and uncomment the following code to enable the ICommDlgBrowser2 interface

Don't return ICommDlgBrowser2 when your QueryInterface is asked for it, it will break your app :(

However, my implementation works now. Thanks, dahall! 👍

dahall commented 5 years ago

Until I release the code and have you test the released package, I'm going to leave this open. Thanks for the note on ICommDlgBrowser2. You're using ICommDlgBrowser3 successfully even though it inherits from ICommDlgBrowser2?

dahall commented 5 years ago

Another question, I'm using IServiceProvider to expose ICommDlgBrowser3 to my test app, but the ICommDlgBrowser3 methods are never getting called. Any guesses as to the reason? What are you doing other than inheriting from ICommDlgBrowser3 from your form?

tajbender commented 5 years ago

Yes, afaik this is default behavior. At least I've never seen that any of the ICDB2/ICDB3 methods is called by Windows Shell, only ICDB(1) to be honest.

Maybe if you use an IShellView interface, but with this default implementation of ExplorerBrowser component under Windows 7 and Windows 10 I've never seen any call to ICDB2 or ICDB3-methods.

dahall commented 5 years ago

Just an update: I'm fighting that same Access Violation exception. I have changed so many things I'm starting to lose track. So the update is "no progress". I'm working on it a bit each day, so I hope to have something working soon. IExplorerBrowser.Initialize is where I'm seeing the exception.

tajbender commented 5 years ago

The strange thing is, I'm totally confused where to search for. Sometimes my code seems to work fine, then I change something very far away, totally unrelated to that part and it crashes the first time it starts. Launched again it works fine again?!?

However, currently I'm continuing on my old codebase again to get other things done. So it's no more priority for me at the moment, so I recommend you should shift your focus, too. Perhaps I'll have time for further investigations later.

Thank you very much!

dahall commented 5 years ago

Pushed a much better tested assembly today to NuGet (2.1.2) where I've duplicated the samples from Microsoft to ensure the interfaces and structures are working for those exposed from Shell32. Hope this helps your issues. If not, let me know and I'll share the test code I've used.

tajbender commented 5 years ago

Thank you very much!

dahall commented 5 years ago

I decided to publish that work I've done on the samples. It is at https://github.com/dahall/WinClassicSamplesCS.