khanhas / AppVolumePlugin

Plugin for Rainmeter: Get and control apps volume, peak.
GNU General Public License v3.0
33 stars 3 forks source link

Rainmeter crashes after unloading of skin with this plugin #5

Closed d-uzlov closed 5 years ago

d-uzlov commented 5 years ago

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? https://github.com/khanhas/AppVolumePlugin/blob/a196e9271809498f8cfbfd338cc5833c3a2041bd/AppVolume/AppVolume.cpp#L45 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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

d-uzlov commented 5 years ago

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 commented 5 years ago

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.