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

Rainmeter crashes after unloading of skin with this plugin #5

Closed
d-uzlov opened this issue Oct 11, 2018 · 6 comments
Closed

Rainmeter crashes after unloading of skin with this plugin #5

d-uzlov opened this issue Oct 11, 2018 · 6 comments

Comments

@d-uzlov
Copy link

d-uzlov commented Oct 11, 2018

How to reproduce:
Load 2 skins with AppVolumePlugin (for example, 2 instances of your example skin), unload 1 of them.

Are you sure that you want to release static field "pEnumerator" on every parent measure unload?

SAFE_RELEASE(pEnumerator);

Null pointer dereferencing is probably the cause of the crash because static pEnumerator is null after unloading of a skin containing this plugin. Maybe it should not be static?

Also, although this is not directly related to the issue, I think that CoCreateInstance() in InitializeCOM() leads to memory leak because if pEnumerator is initialized it is not released before second initialization.
Another memory leak is probably happening because CoUninitialize should be called for every successful CoInitialize but now you call it only once per plugin lifetime.

@khanhas
Copy link
Owner

khanhas commented Oct 11, 2018

Thank you for taking time reading the code! I really appreciate it.
And you're right, I shouldn't release device enumerator in measure destructor. It will be released when no parent measure is loaded (parentCollection.size() == 0).
I also added nullptr check before CoCreateInstance.
Download new version here: https://github.com/khanhas/AppVolumePlugin/releases/tag/1.2.1

@d-uzlov
Copy link
Author

d-uzlov commented Oct 11, 2018

Thank you for taking time reading the code!

I'm glad to help!

Download new version here

Unfortunately, I can't start Rainmeter with this version. It crashes before first redraw cycle.

P.S. Sorry for reporting random issues in this thread, but I noticed that field "LegalCopyright" in your dlls contain "??" instead of "©" for me, while all other plugins I have shows ©.

@khanhas
Copy link
Owner

khanhas commented Oct 11, 2018

Haha, I made a mistake.
I checked pEnumerator != nullptr instead of == nullptr
Fixed: https://github.com/khanhas/AppVolumePlugin/releases/tag/1.2.2

About the copyright symbol, I used speaker emoji instead of © so it's maybe not available in your system.
Are you using Windows 7? Since if you use Windows 10, it will show like this:

image

image

Eh, maybe I should change to © in next patch

@d-uzlov
Copy link
Author

d-uzlov commented Oct 11, 2018

Now everything works as intended.
Thank you for quick response.

@d-uzlov d-uzlov closed this as completed Oct 11, 2018
@d-uzlov
Copy link
Author

d-uzlov commented Oct 11, 2018

Are you using Windows 7

No, I'm using win10. I think that it can be related to default charset: I use cp1251. But I thought that Unicode is used for this (and two question mark suggests that something like utf-16 is used).
Maybe the problem is that I don't have this emoji? I don't use them, so I don't know if they work correctly on my machine.

@d-uzlov
Copy link
Author

d-uzlov commented Oct 11, 2018

Ok, I think I found the issue with this symbol. I used different software for checking this information (plugin for total commander), and it apparently can't render emojis. Rainmeter and explorer show it as in your screenshot. Sorry for false report.

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

No branches or pull requests

2 participants