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

Crash in WinToast destructor #44

Open rrrado opened 5 years ago

rrrado commented 5 years ago

Sometimes users reports crash while closing the application. I cannot reproduce the problem by myself it seems to be random and not very often. The call stack looks always like this:

wpnapps.dll!Microsoft::WRL::ComPtr<IStream>::InternalRelease() Line 176 C++
[Inline Frame] wpnapps.dll!Microsoft::WRL::ComPtr<Windows::Data::Xml::Dom::IXmlDocument>::{dtor}()  C++
wpnapps.dll!BaseNotification::~BaseNotification() Line 33   C++
[External Code] 
wpnapps.dll!Microsoft::WRL::Details::RuntimeClassImpl<Microsoft::WRL::RuntimeClassFlags<1>,1,1,0,Microsoft::WRL::Implements<Windows::UI::Notifications::IToastNotification,Windows::UI::Notifications::IToastNotification2,Windows::UI::Notifications::IToastNotification3,Windows::UI::Notifications::IToastNotification4,IInspectable,Windows::UI::Notifications::Internal::IPostNotification,Windows::UI::Notifications::Internal::IToastNotificationInternal>,BaseNotification,Microsoft::WRL::FtmBase>::Release() Line 2013    C++
[External Code] 
[Inline Frame] s.exe!std::_Tree<std::_Tmap_traits<__int64,Microsoft::WRL::ComPtr<ABI::Windows::UI::Notifications::IToastNotification>,std::less<__int64>,std::allocator<std::pair<__int64 const ,Microsoft::WRL::ComPtr<ABI::Windows::UI::Notifications::IToastNotification> > >,0> >::_Tidy()  C++
[Inline Frame] s.exe!std::_Tree<std::_Tmap_traits<__int64,Microsoft::WRL::ComPtr<ABI::Windows::UI::Notifications::IToastNotification>,std::less<__int64>,std::allocator<std::pair<__int64 const ,Microsoft::WRL::ComPtr<ABI::Windows::UI::Notifications::IToastNotification> > >,0> >::{dtor}() C++
s.exe!WinToastLib::WinToast::~WinToast() Line 355   C++
s.exe!_execute_onexit_table::__l22::<lambda>() Line 198 C++
s.exe!__crt_seh_guarded_call<int>::operator()<<lambda_995298e7d72eb4c2aab26c0585b3abe5>,int <lambda>(void) &,<lambda_293819299cbf9a7022e18b56a874bb5c> >(__acrt_lock_and_call::__l3::<lambda_995298e7d72eb4c2aab26c0585b3abe5> && setup, _execute_onexit_table::__l22::int <lambda>(void) & action, __acrt_lock_and_call::__l4::<lambda_293819299cbf9a7022e18b56a874bb5c> && cleanup) Line 199  C++
s.exe!__acrt_lock_and_call<int <lambda>(void) >(const __acrt_lock_id lock_id, _execute_onexit_table::__l22::int <lambda>(void) && action) Line 882  C++
s.exe!_execute_onexit_table(_onexit_table_t * table) Line 221   C++
s.exe!common_exit(const int return_code, const _crt_exit_cleanup_mode cleanup_mode, const _crt_exit_return_mode return_mode) Line 215   C++
s.exe!exit(int return_code) Line 282    C++
mohabouje commented 5 years ago

@rrrado do you know any steps that may help me to reproduce the problem? I am not able to reproduce it.

I guess is not the same problem but a few months ago, we had a similar problem. The application was calling WinToast::showToast and then interrupted because of a call to std::exit or std::terminate. After changing the behavior, the crash no longer appeared. We did not figure out why this thing was happening, but the code was crashing internally in the Microsoft API itself.

I will take a look and try to figure out how to deal with this issue.

rrrado commented 5 years ago

@mohabouje I'm sorry, I cannot reproduce it. I've tried to close the application while notification was shown, but it didn't crash. I don't really know how does WinToast work internally (I was never working with WinRT) but according to call stack it looks like there were some outdated notification interfaces in the _buffer at the time of application exit. I guess crashing the application in such case when program is exiting is pointless, wouldn't it be safer just to clear the _buffer in the destructor explicitly in the try/catch block and ignore all exceptions?

kenkit commented 5 years ago

I can reproduce this bug in the console-example.exe I run the program with the following params in vscode

       "program": "${workspaceFolder}/bin/Debug/console-example.exe",
            "args": ["--text", "haah", "--expirems", "1000"]

When the notification expires the program crushes. Bug occurs in the default visualstudio build [VS2017] and cmake builds. It could be as a result of vs2017 or windows api change

kenkit commented 5 years ago

It's possible that the handle is expiring before the toast has expired hence the crush

attilab97 commented 4 years ago

Any updates on this issue?

FigBug commented 4 years ago

For me, it's crashing if you exit the app with a toast on screen. I keep a list of toasts on screen and hide them in my destructor. That seems to work around the crash.

void hideAllOSNotifications ()
{
    auto& instance = *WinToastLib::WinToast::instance ();

    if ( instance.isInitialized () )
    {
        for ( auto id : visibleToasts )
        {
            instance.hideToast ( id );
        }

        instance.clear ();

        visibleToasts.clear ();
    }
}
attilab97 commented 4 years ago

I did exactly what you did and it keeps crashing. I have attached pictures with the crash message and the call stack. Thank you. break call_stack

rrrado commented 4 years ago

I've modified destructor to swallow the access violation. It's dirty but it seems it works for me.

void WinToast::SilentClear()
{
    __try
    {
        clear();
    }
    __except (EXCEPTION_EXECUTE_HANDLER)
    {        
    }
}

WinToast::~WinToast() {

    SilentClear(); // ignore crash when program is exiting while notification is visible

    if (_hasCoInitialized) {
        CoUninitialize();
    }
}
sevoster commented 4 years ago

I use WinToast along with Qt and this crash occurs on exit because destructor is called after destruction of QApplication. Fixed it by deinitializing WinToast before qApp quits.

ChrisDrozdowski commented 4 years ago

Hi,

For what it is worth, I had a very similar COM-based singleton class (completely different COM lib) that called CoUninitialize in its destructor and it suffered from similar issues. I used it in console apps and believe that it MAY be related to the destructor being called from a different thread than the one in which it was created (even in a single-threaded app). But I'm not totally sure that was why it happened.

I tried everything I could think of or research and concluded the only alternative was to, like sevoster, call a deinitializer function from the same thread that calls ConInitialize. I also found I had to sleep for 250-500 ms after calling CoInitialize or I sometimes got an error.

I just thought I'd share this.

rrrado commented 4 years ago

I think I've found problem - at least in my case. I have MFC application and I'm calling AfxOleTerm() which calls CoUninitialize internally in my ExitInstance(). Unfortunately ~WinToast() destructor is called AFTER this so it crashes. I'm not sure it is fortunate to have uninitialization in singleton's destructor, because it is typically called very late after typical application already called CoUninitialize() IMHO. I've added cleanup code WinToast::instance()->clear() before my AfxOleTerm() and it seems it is not crashing so far.

TheMaverickProgrammer commented 4 years ago

rrrado's solution is working for me as well.