search

mohabouje / WinToast

WinToast is a lightly library written in C++ which brings a complete integration of the modern toast notifications of Windows 8 & Windows 10. Toast notifications allows your app to inform the users about relevant information and timely events that they should see and take action upon inside your app, such as a new instant message, a new friend request, breaking news, or a calendar event.
MIT License
688 stars 127 forks source link

_instance is set to null and then deleted #22

Closed PhilLab closed 5 years ago

PhilLab commented 6 years ago

The destructor Sets to null and then deletes, which makes no sense. Aside from that, I don't understand when the destructor is ever called and if the (instance) destructor should really delete the static member

mohabouje commented 6 years ago

You are totally right! I did a quick fix: (https://github.com/mohabouje/WinToast/commit/f020f6f2c318700765024454cc32f3c6be08ad8c). Now, the code should run correctly and the destructor is called at the end of the program execution.

PhilLab commented 6 years ago

@mohabouje I applied your changes and now my program crashes on exit in the WinToast destructor at _xmlDocument.Reset();. But only if I previously showed a notification somewhere. If not, the program exits normally. Do you have the same problem?

PhilLab commented 6 years ago

Aside from that, the .Reset() is not needed as the object are smart pointers which release on their destructor. But unfortunately the _xmlDocument destructor crashes

PhilLab commented 6 years ago

@mohabouje I had to revert the change because otherwise this library's current HEAD is not usable anymore. Can you confirm that it workes on your machine?

mohabouje commented 6 years ago

I will try to figure out how to replicate your problems. I'm not able to debug the problema, I use the library in different projects and it works fine.

mohabouje commented 6 years ago

Finally, I find the problem. I was looking in to the new Microsoft Examples and how they fix it. I just limited the scope of the different ComPtr. I did some clean and tested it in Windows 8 & 10. It seems to work fine, Can you confirm @PhilLab ??

PhilLab commented 6 years ago

Thanks for looking into it. How did you make it work if initialize() always return false?.

If I fix this (setting to return true), I can't show notifications from different threads than the one I first instantiated the lib with - unfortunately, this is necessary in my architecture.

mohabouje commented 6 years ago

It was a mistake to return a false in initialise function. It was reported here, and fixed https://github.com/mohabouje/WinToast/issues/24

mohabouje commented 6 years ago

About the problem you explain, I had the same problem in the past, what I did is to create different instance using the normal constructor and avoiding the singleton. If all the different threads use the same AUMI, then it should work without problem.