Difegue / Stylophone

A pretty cool™️ MPD client in .NET. Based on MpcNET.
https://tvc-16.science/stylophone-27.html
MIT License
130 stars 10 forks source link

Crash when changes happen to the queue #71

Closed hlbstd closed 11 months ago

hlbstd commented 1 year ago

Hi,

I've been trying to use Stylophone on windows but the app keeps on crashing whenever I try to skip a song, or modify a playlist in a way that removes items. I've build a debug version to investigate and here is what happens on my system:

  1. contrary to the comment at QueueViewModel.cs:209 plchanges does not necessarily give the full list of files. For instance when skipping on my system the response always contains a single song which is the newly playing song. The logic that follows is then invalid.
  2. The removal of the chosen songs then crashes the ui. I'm not sure what could be causing this crash since the list of song is still valid from an UI perspective.

Here is the stack of the crash:

    KernelBase.dll!RaiseFailFastException()    Unknown
    combase.dll!RoFailFastWithErrorContextInternal2(HRESULT hrError, unsigned long cStowedExceptions, _STOWED_EXCEPTION_INFORMATION_V2 * * aStowedExceptionPointers) Line 1468  C++
    [Inline Frame] Windows.UI.Xaml.dll!std::vector<float,std::allocator<float>>::_Xrange() Line 1728    C++
    Windows.UI.Xaml.dll!std::vector<float,std::allocator<float>>::at(const unsigned __int64 _Pos) Line 1494 C++
    Windows.UI.Xaml.dll!DirectUI::Components::Moco::StackingLayoutStrategyImpl::RegisterSize(int index, bool isHeader, float newSize) Line 758  C++
    Windows.UI.Xaml.dll!DirectUI::Components::Moco::StackingLayoutStrategyImpl::GetContainerBounds(int indexInItems, int containerDesiredSize, Windows::Foundation::Size referenceInformation, Windows::UI::Xaml::Controls::LayoutReference) Line 804   C++
    Windows.UI.Xaml.dll!DirectUI::Components::Moco::StackingLayoutStrategyImpl::GetElementBounds(Windows::UI::Xaml::Controls::ElementType elementType, int elementIndex, Windows::Foundation::Size containerDesiredSize, Windows::UI::Xaml::Controls::LayoutReference referenceInformation, Windows::Foundation::Rect pReturnValue, Windows::Foundation::Rect *) Line 105   C++
    Windows.UI.Xaml.dll!DirectUI::StackingLayoutStrategy::GetElementBoundsImpl(Windows::UI::Xaml::Controls::ElementType elementType, int elementIndex, Windows::Foundation::Size containerDesiredSize, Windows::UI::Xaml::Controls::LayoutReference referenceInformation, Windows::Foundation::Rect pReturnValue, Windows::Foundation::Rect *) Line 79  C++
    Windows.UI.Xaml.dll!DirectUI::StackingLayoutStrategyGenerated::GetElementBounds(Windows::UI::Xaml::Controls::ElementType elementType, int elementIndex, Windows::Foundation::Size containerDesiredSize, Windows::UI::Xaml::Controls::LayoutReference referenceInformation, Windows::Foundation::Rect windowConstraint, Windows::Foundation::Rect * pResult) Line 177    C++
    Windows.UI.Xaml.dll!DirectUI::ModernCollectionBasePanel::OnItemRemoved(int nIndex, bool itemRemovedForGroupReset) Line 1662 C++
    Windows.UI.Xaml.dll!DirectUI::ModernCollectionBasePanel::NotifyOfItemsChangedImpl(Windows::Foundation::Collections::IVectorChangedEventArgs * e) Line 1251  C++
    Windows.UI.Xaml.dll!DirectUI::ModernCollectionBasePanelGenerated::NotifyOfItemsChanged(Windows::Foundation::Collections::IVectorChangedEventArgs * pE) Line 915 C++
    Windows.UI.Xaml.dll!DirectUI::ItemsControl::OnItemsChanged(Windows::Foundation::Collections::IVectorChangedEventArgs * pArgs) Line 1111 C++
    Windows.UI.Xaml.dll!DirectUI::Selector::OnItemsChanged(Windows::Foundation::Collections::IVectorChangedEventArgs * e) Line 1375 C++
    Windows.UI.Xaml.dll!DirectUI::ListViewBase::OnItemsChanged(Windows::Foundation::Collections::IVectorChangedEventArgs * pArgs) Line 200  C++
    Windows.UI.Xaml.dll!DirectUI::ItemsControl::OnItemsChangedImpl(IInspectable * e) Line 1125  C++
    Windows.UI.Xaml.dll!DirectUI::ItemsControlGenerated::OnItemsChanged(IInspectable * pE) Line 683 C++
    Windows.UI.Xaml.dll!DirectUI::ItemsControlGenerated::OnItemsChangedProtected(IInspectable * pE) Line 704    C++
    [Inline Frame] Windows.UI.Xaml.dll!std::_Func_class<long,Windows::Foundation::Collections::IObservableVector<Windows::Foundation::DateTime> *,Windows::Foundation::Collections::IVectorChangedEventArgs *>::operator()(Windows::Foundation::Collections::IObservableVector<Windows::Foundation::DateTime> * <_Args_0>, Windows::Foundation::Collections::IVectorChangedEventArgs * <_Args_1>) Line 869  C++
    Windows.UI.Xaml.dll!ctl::event_handler_base<Windows::Foundation::Collections::VectorChangedEventHandler<Windows::Foundation::DateTime>,Windows::Foundation::Collections::IObservableVector<Windows::Foundation::DateTime>,Windows::Foundation::Collections::IVectorChangedEventArgs,DirectUI::DateTimeVectorChangedTraits>::Invoke(Windows::Foundation::Collections::IObservableVector<Windows::Foundation::DateTime> * pSender, Windows::Foundation::Collections::IVectorChangedEventArgs * pArgs) Line 38 C++
    Windows.UI.Xaml.dll!DirectUI::CEventSourceBase<DirectUI::IUntypedEventSource,Windows::Foundation::Collections::VectorChangedEventHandler<IInspectable *>,Windows::Foundation::Collections::IObservableVector<IInspectable *>,Windows::Foundation::Collections::IVectorChangedEventArgs>::Raise(Windows::Foundation::Collections::IObservableVector<IInspectable *> * pSource, Windows::Foundation::Collections::IVectorChangedEventArgs * pArgs) Line 307   C++
    Windows.UI.Xaml.dll!DirectUI::ItemCollection::OnCollectionChanged(Windows::Foundation::Collections::IObservableVector<IInspectable *> * pSender, Windows::Foundation::Collections::IVectorChangedEventArgs * e) Line 410    C++
    [Inline Frame] Windows.UI.Xaml.dll!DirectUI::ItemCollection::UpdateItemsSourceList::__l23::<lambda_333c454cfd67f0d9d59f18ca318bd614>::operator()(IInspectable * sender, Windows::Foundation::Collections::IVectorChangedEventArgs * args) Line 368  C++
    [Inline Frame] Windows.UI.Xaml.dll!std::invoke(DirectUI::ItemCollection::UpdateItemsSourceList::__l23::<lambda_333c454cfd67f0d9d59f18ca318bd614> &) Line 1488   C++
    [Inline Frame] Windows.UI.Xaml.dll!std::_Invoker_ret<long,0>::_Call(DirectUI::ItemCollection::UpdateItemsSourceList::__l23::<lambda_333c454cfd67f0d9d59f18ca318bd614> &) Line 660   C++
    Windows.UI.Xaml.dll!std::_Func_impl_no_alloc<<lambda_333c454cfd67f0d9d59f18ca318bd614>,long,Windows::Foundation::Collections::IObservableVector<IInspectable *> *,Windows::Foundation::Collections::IVectorChangedEventArgs *>::_Do_call(Windows::Foundation::Collections::IObservableVector<IInspectable *> * && <_Args_0>, Windows::Foundation::Collections::IVectorChangedEventArgs * && <_Args_1>) Line 822 C++
    [Inline Frame] Windows.UI.Xaml.dll!std::_Func_class<long,Windows::Foundation::Collections::IObservableVector<Windows::Foundation::DateTime> *,Windows::Foundation::Collections::IVectorChangedEventArgs *>::operator()(Windows::Foundation::Collections::IObservableVector<Windows::Foundation::DateTime> * <_Args_0>, Windows::Foundation::Collections::IVectorChangedEventArgs * <_Args_1>) Line 869  C++
    Windows.UI.Xaml.dll!ctl::event_handler_base<Windows::Foundation::Collections::VectorChangedEventHandler<Windows::Foundation::DateTime>,Windows::Foundation::Collections::IObservableVector<Windows::Foundation::DateTime>,Windows::Foundation::Collections::IVectorChangedEventArgs,DirectUI::DateTimeVectorChangedTraits>::Invoke(Windows::Foundation::Collections::IObservableVector<Windows::Foundation::DateTime> * pSender, Windows::Foundation::Collections::IVectorChangedEventArgs * pArgs) Line 38 C++
    Windows.UI.Xaml.dll!DirectUI::TrackerEventSource<Windows::Foundation::Collections::VectorChangedEventHandler<IInspectable *>,Windows::Foundation::Collections::IObservableVector<IInspectable *>,Windows::Foundation::Collections::IVectorChangedEventArgs>::Raise(Windows::Foundation::Collections::IObservableVector<IInspectable *> * pSource, Windows::Foundation::Collections::IVectorChangedEventArgs * pArgs) Line 572   C++
    [Inline Frame] Windows.UI.Xaml.dll!DirectUI::BindableObservableVectorWrapper::RaiseVectorChanged(Windows::Foundation::Collections::IVectorChangedEventArgs *) Line 286  C++
    Windows.UI.Xaml.dll!DirectUI::BindableObservableVectorWrapper::RaiseVectorChanged(Windows::Foundation::Collections::CollectionChange action, unsigned int index) Line 274   C++
    Windows.UI.Xaml.dll!DirectUI::BindableObservableVectorWrapper::ProcessCollectionChange(Windows::UI::Xaml::Interop::INotifyCollectionChangedEventArgs * pArgs) Line 225  C++
    [Inline Frame] Windows.UI.Xaml.dll!std::_Func_class<long,IInspectable *,Windows::UI::Xaml::Interop::INotifyCollectionChangedEventArgs *>::operator()(IInspectable * <_Args_0>, Windows::UI::Xaml::Interop::INotifyCollectionChangedEventArgs * <_Args_1>) Line 869  C++
    Windows.UI.Xaml.dll!ctl::event_handler_base<Windows::UI::Xaml::Interop::INotifyCollectionChangedEventHandler,IInspectable,Windows::UI::Xaml::Interop::INotifyCollectionChangedEventArgs,DirectUI::CollectionChangedTraits>::Invoke(IInspectable * pSender, Windows::UI::Xaml::Interop::INotifyCollectionChangedEventArgs * pArgs) Line 38   C++
    Windows.UI.Xaml.dll!ctl::event_handler_base<Windows::UI::Xaml::Interop::INotifyCollectionChangedEventHandler,IInspectable,Windows::UI::Xaml::Interop::INotifyCollectionChangedEventArgs,DirectUI::CollectionChangedTraits>::Invoke(IInspectable * pSender, Windows::UI::Xaml::Interop::INotifyCollectionChangedEventArgs * pArgs) Line 27   C++
    [External Code] 
>   Stylophone.Common.dll!Stylophone.Common.ViewModels.QueueViewModel.MPDConnectionService_QueueChanged.AnonymousMethod__2() Line 217   C#

Replacing the RemoveRange with a for loop of RemoveAt seems to solve the crash.

I've not seen this issue reported so maybe there's something on my setup that triggers both weird behaviors. The mpd server protocol version is 0.23.5 and it's the android version of the app.

Difegue commented 1 year ago

👋 Thanks for the detailed report - I haven't updated my MPD test server in a little while so I'm still running 0.22.0, but there's nothing in the changelogs since then that seem to affect plchanges.
I don't think skipping should trigger a queue changed/playlist subsystem event, unless maybe you have Consume enabled?

Number 2 looks like a WinUI bug -- I've seen a crash report in AppCenter recently that might come from this: image

hlbstd commented 1 year ago

Skipping a song always trigger this event for me, with consume enabled or not. With consume enabled the behaviour actually matches the comment (I receive the songs remaining in the playlist). I think this is not the only instance of me receiving this event more often than intended by the app. I have another issue when playing an album for instance, the callback is triggered at least twice and is run in parallel which also messes with the list.

An easy fix on my side is to use the playlistinfo command and run ReplaceRange on the whole queue each time but it makes the ui refresh a lot more than it should I think.

Putting aside the what triggers the callback, shouldn't the result of plchanges be handled a bit differently anyway ? Since the command gives the new offsets of the modified elements, couldn't they be used to find out which elements have actually been removed/added/shuffled without presuming of the result and with minimal modification to the list ? I don't know much about xaml/winui so maybe this wouldn't be beneficial to minimizes changes to the UI.

hlbstd commented 1 year ago

I think I've found the cause of those additional notifications, it's the proxy database plugin, aka "satellite mode". I've switched to a local db and the application now works as expected.

This issue is solved as far as I'm concerned, I'll leave it open in case you want to handle this particular setup in app.

ebsbow commented 1 year ago

For those who have the app crashing random or after some minutes, you could try to reset the app data.

Richt-click the icon and select App-Settings(..) Then select Reset or try a repair.

This might help in some situations.

It seems to fix my issue.

I love this app. Thanks for the work!

holsta commented 1 year ago

I have this issue on Windows, and reported it via the Microsoft Store. In Settings, Stylophone is allowed to send crash and analytics reports. Are you getting those reports?

I am using the local playback mode: All my FLAC files are on the NAS and mpd streams audio to Stylophone.

It seems Stylophone survives longer than one song if I load up the queue with music and change to one of the other views.

Difegue commented 1 year ago

Not seeing anything in AppCenter sadly -- Native crashes tend to escape the unhandled exception catch for some reason, this is one reason that makes me want to eventually move away from .NET Native when the tooling gets there. 🤔

Does the issue still happen with local playback disabled?

holsta commented 1 year ago

Does the issue still happen with local playback disabled?

I tried this just now, and yes, Stylophone crashes when I skip to the next song even with Local Playback disabled. I would assume you would be bogged down in bug reports if every Windows customer had these problems, so I wonder if it's something local to my system. Could you point me to the code that is called at queue updates?

Difegue commented 1 year ago

This message skipped by me for some reason - sorry about that!
The main logic for queue updates can be found starting here: https://github.com/Difegue/Stylophone/blob/6d0e2f461cf68c6b657268c5b5ff0999493dea76/Sources/Stylophone.Common/ViewModels/QueueViewModel.cs#L185

This is usually called when the idle polling reports a queue change.

Difegue commented 11 months ago

I've revamped the queue update mechanism in 2.6 (again), so I'll tentatively close this for now. Feel free to reopen if it still happens!