Dijji / FileMeta

Enable Explorer in Vista, Windows 7 and later to see, edit and search on tags and other metadata for any file type
Microsoft Public License
766 stars 68 forks source link

Crash when adding handler to SolidWorks formats #69

Open beezerlm opened 5 years ago

beezerlm commented 5 years ago

FileMeta crashed when adding custom profile to .sldprt .sldasm and .slddrw file extensions. These are the only formats that crashed for me out of about 20 that I applied.

Dijji commented 5 years ago

I assume that it is the Association Manager that is crashing. Of course, it shouldn't be doing so. The probability is that one of the registry operations that it is attempting is failing, and the program is not catching the error. I need to change the code so that you get an error message instead, ideally with some indication of what is failing.

In the meantime, given that it is only failing on a specific group of extensions, all of which seem to be associated with a particular program, which I imagine created the entries for those extensions in the first place, the most likely explanation is that the program has restricted access to the keys that it has created. What happens if you go into the registry editor and try to make the modifications that the Association Manager would?

Dijji

beezerlm commented 5 years ago

Yes it was Association Manager that crashed.

I looked at the registry location HKEY_CLASSES_ROOT\SystemFileAssociations\ to see if I can manually make the changes, but these file extensions are not even listed here?

Dijji commented 5 years ago

The extensions appear in Association Manager if and only if they appear under HKEY_CLASSES_ROOT, for example , HKEY_CLASSES_ROOT\.sldprt

The error could be occurring on any of the registry locations that Association Manager modifies, as described in the documentation.

Dijji

beezerlm commented 5 years ago

HKEY_CLASSES_ROOT\.sldprt - exists. HKEY_CLASSES_ROOT\SystemFileAssociations\.sldprt - does not exist.

Are we on the same page or am I missing something?

Dijji commented 5 years ago

I'm not sure. Maybe we should just follow the steps that the Association Manager takes and see how far it is getting. First, do the extensions involved have existing handlers? Secondly, what is the default value of the HKEY_CLASSES_ROOT\.sldprt key?

If you want to follow along in the code, we're looking at the SetupHandlerForExtension method in Extension.cs

Dijji

beezerlm commented 5 years ago

Edit: Just downloaded the wixproj VS extension and was able to reload the project without errors.

First, do the extensions involved have existing handlers?

Yes, these files have an existing handler

Secondly, what is the default value of the HKEY_CLASSES_ROOT.sldprt key?

SLDPRT_auto_file

If you want to follow along in the code, we're looking at the SetupHandlerForExtension method in Extension.cs

I tried to open the solution in VS2017 so I could step through the program but got this message:

Unsupported
This version of Visual Studio is unable to open the following projects. The project types may not be installed or this version of Visual Studio may not support them. 
For more information on enabling these project types or otherwise migrating your assets, please see the details in the "Migration Report" displayed after clicking OK.
     - Setup, "I:\Visual_Studio_2017\Projects\FileMeta-master\FileMeta-master\Setup\Setup.wixproj"

Setup\Setup.wixproj: The application which this project type is based on was not found.

Migration

Is this project required for my needs or do you know how to migrate it to VS2017?

beezerlm commented 5 years ago

Here is a screenshot of the unhandled exception:

NullException

Let me know if you need more info.

Dijji commented 5 years ago

No, that is quite clear. I believe that the simple fix is to test for the case where there is no existing 32-bit handler, adding a single line to the code, which becomes: '

   else // Foreign
                {
                    var temp = handler.GetValue(null);
                    handler.SetValue(null, OurPropertyHandlerGuid32);
                    if (temp != null)
                        handler.SetValue(ChainedValueName, temp);
                }

' Would it be possible for you to try this in your development environment?

The full fix is actually more complicated because the code that removes the handler is also broken in this case, in that it will not remove the 32-bit File Meta handler that will be set up with the first fix in place, but the fix is more than a single line. I will need to rerun my tests to make sure that I haven't messed up anything else unintentionally before checking that in.

Assuming that you can apply the first fix, could you live with your own private version for now? As this is a very small fix for a full release, it will probably be a while before I ship it in that form, but I could offer you a private release of the Association Manager in the meantime.

Dijji

Dijji commented 5 years ago

Closed by accident, sorry

beezerlm commented 5 years ago

Applied the null check. Looks like we may need to add another one here:

NullException2

Yes a private release would be fine.

Dijji commented 5 years ago

I think that's just a reflection of the same unhandled exception further up the call stack (I'm assuming that this is recurring as a result of the same Add Handler action)

beezerlm commented 5 years ago

OK so I got to the next step:

NullException3

I added a null exception for that one. Now I get this which has me a bit puzzled?

NullException4

Not sure where to go from here.

Dijji commented 5 years ago

That looks like the source is at a different version to the executable, because the source shown cannot raise the exception that you are getting. The source has the fix, but it looks like the executable does not. With the original one line source change in and compiled, I think that all the exceptions that you are reporting should go away.

Dijji

beezerlm commented 5 years ago

That looks like the source is at a different version to the executable

How does this happen? Just curious because I have never seen this before. Is the source calling a previously compiled exe file?

Dijji commented 5 years ago

I found that simply starting the debugger with the Association Manager as the start-up project does not build it. It is necessary to right click the project in Solution Explorer and build it explicitly.

I have now reproduced the problem and tested the fix against the reproduction, and it works. Now I need to test somewhat more broadly to check for regressions.

Dijji

Dijji commented 5 years ago

I have now completed testing and posted the updated executables in the release "Patch to v1.5 for issue #69". The updated sources have also been checked in. Please give this a go and let me know what you find.

beezerlm commented 5 years ago

I have applied the patch and all looks good except one thing. When applying a custom profile to .sldasm file extension, it appears to apply fine in the Association Manager, but the new properties don't show up in Explorer. I tried restarting explorer, and rebooting the system but no change. I looked at the registry entries and they seem to match the .sldprt extension which works fine. Any ideas?

Dijji commented 5 years ago

I tried to reproduce the issue by continuing to configure the fake extension I used to reproduce the crash, but no luck. Everything works perfectly in an irritating fashion. So no ideas from this end at the moment.

beezerlm commented 5 years ago

Taking a wild stab here. Could it have anything to do with the "chained" Solidworks propertyHandler having a ThreadingModel of "apartment", while FileMeta's property handler ThreadingModel is "both"?

Dijji commented 5 years ago

This shouldn't be the case. The worst that should happen is that the property handler gets an error opening the chained property handler. However, it is coded to ignore this case by just forgetting about reading chained properties.

But if you're still in doubt, it would be easy to double check by temporarily renaming the value pointing to the chained property handler, so that it is not picked up at all.