Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exception Thrown in Destructors Causing Program Crash #31

Closed
mikasoukhov opened this issue Aug 22, 2024 · 8 comments · Fixed by #32
Closed

Exception Thrown in Destructors Causing Program Crash #31

mikasoukhov opened this issue Aug 22, 2024 · 8 comments · Fixed by #32

Comments

@mikasoukhov
Copy link

I've encountered an issue in WmiLight where destructors ( https://github.com/MartinKuschnik/WmiLight/blob/master/WmiLight/Wbem/IUnknown.cs#L22 ) are throwing exceptions, leading to the crash of the entire program. While this doesn't affect the original error in my case (no access to the native DLL), causing the entire program to crash is not ideal. This behavior should be addressed to prevent unnecessary crashes, even when the underlying error isn't critical.

@MartinKuschnik
Copy link
Owner

Hi @mikasoukhov, you are right. This should be changed.

I'm currently in vacation. Maybe you can create a PR?

@MartinKuschnik
Copy link
Owner

Version 6.5.1 changes the behavior.

mikasoukhov added a commit to StockSharp/Ecng that referenced this issue Sep 3, 2024
@mikasoukhov
Copy link
Author

I wanted to confirm that the issue is still present. The required file WmiLight.Native.dll is not found (although this might not be directly related to the current issue), and calling ReleaseIUnknown results in an exception, causing the application to crash.

To handle this more gracefully, it would be ideal to wrap the ReleaseIUnknown call in a try/catch block and log the exception details. For instance, using Trace.WriteLine to log the information would be a good approach. Additionally, it would be useful to log any previous errors or incorrect states returned by ReleaseIUnknown, even if it doesn't throw an exception.

I'm not familiar with the entire codebase, but if there are any other finalizers or similar code patterns elsewhere, it would be worth checking those as well to ensure they're handled properly.

@MartinKuschnik
Copy link
Owner

What's the reason that the dll is no longer present?

@mikasoukhov
Copy link
Author

What's the reason that the dll is no longer present?

The missing DLL was my mistake – the program was incorrectly installed, and the runtimes folder was missing. After fixing that, everything worked. However, the application kept crashing before it could even log that a required file was missing. The reason is what I described earlier. Having the app crash like this is bad practice and should be handled better.

@MartinKuschnik MartinKuschnik reopened this Sep 4, 2024
@MartinKuschnik
Copy link
Owner

I'd like to avoid the try/catch block you described.

Therefore i migrated to the methods AddRef, Release and QueryInterface provided by the Marshal class. This eliminates the edge case that the DLL is not longer presend.

This is also similar to the behavior of System.Management (e.g. IWbemClassObjectFreeThreaded).

I hope this solution is also sutable to you.

@mikasoukhov
Copy link
Author

Glad I could help in finding a good solution, and thanks for the fix! Using managed classes is always better than explicit imports, and now everything looks much cleaner. If you don't hear from me in the next few days, that means everything is working well. :)

@MartinKuschnik
Copy link
Owner

Thanks a lot!

mikasoukhov added a commit to StockSharp/Ecng that referenced this issue Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants