CommunityToolkit / Maui

The .NET MAUI Community Toolkit is a community-created library that contains .NET MAUI Extensions, Advanced UI/UX Controls, and Behaviors to help make your life as a .NET MAUI developer easier
https://learn.microsoft.com/dotnet/communitytoolkit/maui
MIT License
2.29k stars 404 forks source link

[BUG] Crash when cleaning up MediaElement on Windows #962

Closed wldevries closed 1 year ago

wldevries commented 1 year ago

Is there an existing issue for this?

Did you read the "Reporting a bug" section on Contributing file?

Current Behavior

  1. Follow the guide at MediaElement announcement blog post to create a very simple page to play a movie from an internet uri.
  2. Start app on net7.0-windows10.0.19041.0
  3. Navigate to the movie page
  4. Browse back

Expected Behavior

App navigates back and resources are cleaned up

Steps To Reproduce

App navigates back and a crash occurs

System.Runtime.InteropServices.COMException (0x80004004): Operation aborted (0x80004004 (E_ABORT))
   at WinRT.ExceptionHelpers.<ThrowExceptionForHR>g__Throw|20_0(Int32 hr)
   at ABI.Windows.Media.Playback.IMediaPlayerMethods.get_Position(IObjectReference _obj)
   at Windows.Media.Playback.MediaPlayer.get_Position()
   at CommunityToolkit.Maui.Core.Views.MediaManager.PlatformUpdatePosition()
   at CommunityToolkit.Maui.Core.Views.MediaManager.UpdateStatus()
   at CommunityToolkit.Maui.Core.Handlers.MediaElementHandler.MapStatusUpdated(MediaElementHandler handler, MediaElement MediaElement, Object args)
   at Microsoft.Maui.Handlers.ElementHandler.Invoke(String command, Object args)
   at CommunityToolkit.Maui.Views.MediaElement.OnTimerTick(Object sender, EventArgs e)
   at Microsoft.Maui.Dispatching.DispatcherTimer.OnTimerTick(DispatcherQueueTimer sender, Object args)
   at WinRT._EventSource_global__Windows_Foundation_TypedEventHandler_global__Microsoft_UI_Dispatching_Dispatcher
QueueTimer__object_.EventState.<GetEventInvoke>b__1_0(DispatcherQueueTimer sender, Object args)
   at ABI.Windows.Foundation.TypedEventHandler`2.Do_Abi_Invoke[TSenderAbi,TResultAbi](Void* thisPtr, TSenderAbi sender, TResultAbi args)

Link to public reproduction project repository

https://github.com/wldevries/MauiMediaElementCrash

Environment

- .NET MAUI CommunityToolkit: 4.0.0
- OS: Windows 10 Build 10.0.19045.2546
- .NET MAUI: no idea.. part of VS 2022 17.4.4
- .NET MAUI MediaElement: 1.0.1

Anything else?

No response

nicholaslambertucci-ett commented 1 year ago

I was creating an issue to report the same problem.

This exception also occurred by running CommunityToolkit.Maui.Sample project: after MediaElement Page was displayed and clicking on "back" buttuon, the System.Runtime.InteropServices.COMException: 'Operation aborted (0x80004004 (E_ABORT))' occurred.

By looking the stack trace

at WinRT.ExceptionHelpers.gThrow|20_0(Int32 hr) at ABI.Windows.Media.Playback.IMediaPlayer3Methods.get_PlaybackSession(IObjectReference _obj) at Windows.Media.Playback.MediaPlayer.get_PlaybackSession() at CommunityToolkit.Maui.Core.Views.MediaManager.Dispose(Boolean disposing) in C:\ETT\ThirdParty\MauiCommunityToolkit\src\CommunityToolkit.Maui.MediaElement\Views\MediaManager.windows.cs:line 277 at CommunityToolkit.Maui.Core.Views.MediaManager.Dispose() in C:\ETT\ThirdParty\MauiCommunityToolkit\src\CommunityToolkit.Maui.MediaElement\Views\MediaManager.windows.cs:line 53 at CommunityToolkit.Maui.Core.Handlers.MediaElementHandler.Dispose(Boolean disposing) in C:\ETT\ThirdParty\MauiCommunityToolkit\src\CommunityToolkit.Maui.MediaElement\Handlers\MediaElementHandler.shared.cs:line 213 at CommunityToolkit.Maui.Core.Handlers.MediaElementHandler.Dispose() in C:\ETT\ThirdParty\MauiCommunityToolkit\src\CommunityToolkit.Maui.MediaElement\Handlers\MediaElementHandler.shared.cs:line 201 at CommunityToolkit.Maui.Core.Handlers.MediaElementHandler.DisconnectHandler(MauiMediaElement platformView) in C:\ETT\ThirdParty\MauiCommunityToolkit\src\CommunityToolkit.Maui.MediaElement\Handlers\MediaElementHandler.windows.cs:line 32 at Microsoft.Maui.Handlers.ViewHandler`2.OnDisconnectHandler(FrameworkElement platformView) at Microsoft.Maui.Handlers.ViewHandler.OnDisconnectHandler(Object platformView) at Microsoft.Maui.Handlers.ElementHandler.DisconnectHandler(Object platformView) at Microsoft.Maui.Handlers.ElementHandler.Microsoft.Maui.IElementHandler.DisconnectHandler() at CommunityToolkit.Maui.Sample.Pages.Views.MediaElementPage.BasePage_Unloaded(Object sender, EventArgs e) in C:\ETT\ThirdParty\MauiCommunityToolkit\samples\CommunityToolkit.Maui.Sample\Pages\Views\MediaElement\MediaElementPage.xaml.cs:line 115 at Microsoft.Maui.Controls.VisualElement.OnUnloadedCore() at Microsoft.Maui.Controls.VisualElement.HandlePlatformUnloadedLoaded() at Microsoft.Maui.Controls.VisualElement.OnWindowChanged(BindableObject bindable, Object oldValue, Object newValue) at Microsoft.Maui.Controls.BindableObject.SetValueCore(BindableProperty property, Object value, SetValueFlags attributes, SetValuePrivateFlags privateAttributes) at Microsoft.Maui.Controls.BindableObject.SetValue(BindableProperty property, Object value, Boolean fromStyle, Boolean checkAccess) at Microsoft.Maui.Controls.BindableObject.SetValue(BindablePropertyKey propertyKey, Object value) at Microsoft.Maui.Controls.Internals.PropertyPropagationExtensions.PropagatePropertyChanged(String propertyName, Element element, IEnumerable children) at Microsoft.Maui.Controls.NavigableElement.OnParentSet() at Microsoft.Maui.Controls.Page.OnParentSet() at Microsoft.Maui.Controls.Element.set_Parent(Element value) at Microsoft.Maui.Controls.BaseShellItem.RemoveLogicalChild(Element element) at Microsoft.Maui.Controls.ShellSection.RemovePage(Page page) at Microsoft.Maui.Controls.ShellSection.d86.MoveNext()

it seems that exception was thrown when accessing the property PlaybackSession of the Windows.Media.Playback.MediaPlayer at line 277 of MediaManager.windows.cs - this happens because Windows.Media.Playback.MediaPlayer has already been disposed at line 29 of MauiMediaElement.windows.cs

To be excact:

// method of MediaManager.windows.cs
protected override void DisconnectHandler(MauiMediaElement platformView)
{
    platformView.Dispose(); // <- MediaPlayer is disposed here
    Dispose(); // <- access to property MediaPlayer.PlaybackSession exception occurred
    base.DisconnectHandler(platformView);
}

// method of MauiMediaElement
public void Dispose()
{
    mediaElement?.MediaPlayer.Dispose();
}

// method of MediaManager.windows.cs
protected virtual void Dispose(bool disposing)
{
    if (disposing)
    {
        if (Player?.MediaPlayer is not null)
        {
            if (displayActiveRequested)
            {
                DisplayRequest.RequestRelease();
                displayActiveRequested = false;
            }

            Player.MediaPlayer.MediaOpened -= OnMediaElementMediaOpened;
            Player.MediaPlayer.MediaFailed -= OnMediaElementMediaFailed;
            Player.MediaPlayer.MediaEnded -= OnMediaElementMediaEnded;
            Player.MediaPlayer.VolumeChanged -= OnMediaElementVolumeChanged;

            if (Player.MediaPlayer.PlaybackSession is not null) // <- exception was thrown here
            {
                Player.MediaPlayer.PlaybackSession.PlaybackRateChanged -= OnPlaybackSessionPlaybackRateChanged;
                Player.MediaPlayer.PlaybackSession.PlaybackStateChanged -= OnPlaybackSessionPlaybackStateChanged;
                Player.MediaPlayer.PlaybackSession.SeekCompleted -= OnPlaybackSessionSeekCompleted;
            }
        }
    }
}

Changing the method DisconnectHandler by calling platformView.Dispose after MediaManager.Dispose, seems to solve the problem, but I can't figure out what this simple change can really involve

// method of MediaManager.windows.cs with method invocation switched
protected override void DisconnectHandler(MauiMediaElement platformView)
{
    //platformView.Dispose();
    Dispose();
    platformView.Dispose();
    base.DisconnectHandler(platformView);
}

I hope this information can help community to solve the problem

Mashedpoturtles commented 1 year ago

Crashes on android too

ne0rrmatrix commented 1 year ago

I had same issue till I did a null check for sender like this.

inside xaml:

<?xml version="1.0" encoding="utf-8" ?>
<ContentPage xmlns="http://schemas.microsoft.com/dotnet/2021/maui"
             xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
              xmlns:viewmodel="clr-namespace:NerdNewsNavigator2.ViewModel.Desktop"
                xmlns:toolkit="http://schemas.microsoft.com/dotnet/2022/maui/toolkit"
             x:Class="NerdNewsNavigator2.View.Desktop.DesktopPlayPodcastPage"
              x:DataType="viewmodel:DesktopPlayPodcastViewModel"
             Shell.NavBarIsVisible="False"
             Title=""
             Unloaded="ContentPage_Unloaded">
    <Shell.BackButtonBehavior>
        <BackButtonBehavior IsEnabled="True" IsVisible="False">
        </BackButtonBehavior>
    </Shell.BackButtonBehavior>

        <Grid>
        <toolkit:MediaElement
               x:Name="mediaElement"
                ShouldAutoPlay="True"
                ShouldShowPlaybackControls="True"
                Source="{Binding Url}"
               />
    </Grid>
</ContentPage>

inside xaml.page.cs

 void ContentPage_Unloaded(object? sender, EventArgs e)
    {
        if (sender is null)
        {
            return;
        }
        // Stop and cleanup MediaElement when we navigate away
        mediaElement.Handler?.DisconnectHandler();
    }

Event args goes off fairly often and you don't want it to trigger accidently. Try adding above null check and see if that helps. I had exact same issue and this fixed it for me. This is example and the relavent code in xaml is:

Unloaded="ContentPage_Unloaded"> 
nicholaslambertucci-ett commented 1 year ago

@ne0rrmatrix I've tried your solution, but I still have the same problem.

I changed the BasePage_Unloaded method body of MediaElementPage.xaml.cs as you suggested

void BasePage_Unloaded(object? sender, EventArgs e)
{
    if (sender is null)
    {
        return;
    }
    // Stop and cleanup MediaElement when we navigate away
    mediaElement.Handler?.DisconnectHandler();
}

but unfortunately in my tests the sender argument is never null and mediaElement.Handler?.DisconnectHandler(); throws the same exception and the reason whould seem the same I've described in my previous comment

exception was thrown when accessing the property PlaybackSession of the Windows.Media.Playback.MediaPlayer at line 277 of MediaManager.windows.cs - this happens because Windows.Media.Playback.MediaPlayer has already been disposed at line 29 of MauiMediaElement.windows.cs

Tested everything with the current state of main branch of this repo

ne0rrmatrix commented 1 year ago

I was able to replicate issue by adding a debug print statement before handler. I think the handler was not being invoked until I added code to the unload event.

void ContentPage_Unloaded(object? sender, EventArgs e)
    {
        if (sender is null)
        {
            return;
        }
        // Next line cause event to fire. Without it I don't get any errors.
        System.Diagnostics.Debug.WriteLine("Event fired.");

        // Stop and cleanup MediaElement when we navigate away
        mediaElement.Handler?.DisconnectHandler();
    }

If I copy paste the code I put earlier in comments I think it simply never fires, at least on Windows. If I had a simple debug statement it fires and app crashes just like you described. I have no idea what is going on. I have been debugging an issue with media element that might be related. But I have to put together a sample and verify the specific cause.

I am going to spend the next day or two working on this. I have to find a work around for my existing project. Right now I am banging my head against the proverbial desk trying to figure out what is wrong.

I have a program that plays podcasts. I added a feature to track playback across a list of shows. Setting the page with media element as transient causes issues.

If I have the page with media element on it set as singleton in MauiProgram.cs I have issues with my code not firing when I switch shows. But If I enable it as singleton media element works flawlessly except for the above crash If I enable unload event. So it works fine if I don't use the code I developed to track shows.

So tracking down a related issue the last few days. I don't know why media element does not want to play nice with MVVM AddTransient. It crashes at third podcast and fails to load show. No errors in debug console. Just fails to load show. It appears to try and load but never completes. Changing to singleton removes issue but I will have to rework code to deal with that.

Might as well try and isolate the issue by creating a separate project and validate the specific problem. That might give me a work around for my needs that can be used by the wider community. I will try and isolate it by creating a project that is bare bones with only code necessary to validate the issue.

I am almost done this starter project for the podcasting app. It has been fun and learning experience. I wanted to try out something new and learn at the same time. I am loving Maui but it can be hard to verify the issue I have are not user errors and genuine bugs.

Taking the time to validate a repeatable flaw in a sample projects has let me not fill up the bug reports with stuff where I have found out later was user error. That has happened about a half dozen times already.

ne0rrmatrix commented 1 year ago
// method of MediaManager.windows.cs with method invocation switched
protected override void DisconnectHandler(MauiMediaElement platformView)
{
  //platformView.Dispose();
  Dispose();
  platformView.Dispose();
  base.DisconnectHandler(platformView);
}

How do I implement this work around? I would like to test it. I have submitted a related bug. If this workaround fixes my issue I can delete my bug report. I am using add transient and it fails to load a third video every time, and it only affects windows. Works fine on Android and iOS. So implementing workaround to see if this bug is the same for me will let me delete my report if it works.

jfversluis commented 1 year ago

Could you maybe try the resulting NuGet from https://github.com/CommunityToolkit/Maui/pull/1002?

That should have this fix.

ne0rrmatrix commented 1 year ago

Could you maybe try the resulting NuGet from #1002?

That should have this fix.

I tried it. It appears to properly dispose of MediaElement. I get no errors. Does not solve my issue of after playing two video going and trying to play a third still fails to open. It tries and platform shows video controls but stream never opens. I have example in my bug report of how to show bug and sample is provided. I have tested the fix and can verify no errors on disconnecting handler. But I still cant play three video's in a row.

My issue only affects Windows. It is still unresolved. But the bug with media element handler disconnect is fixed from this PR.

nicholaslambertucci-ett commented 1 year ago

Could you maybe try the resulting NuGet from #1002?

That should have this fix.

Is this https://pkgs.dev.azure.com/dotnet/696bc9fd-f160-4e97-a1bd-7cbbb3b58f66/_packaging/bdfb41c2-3c30-78f0-b915-ea20f2a39909/nuget/v2/ the url of package source from which i can donwload the resulting Nuget of #1002 ?

jfversluis commented 1 year ago

I don't think so. See the instructions and correct URL here: https://github.com/CommunityToolkit/Maui/wiki/Preview-Packages#pull-requests-

nicholaslambertucci-ett commented 1 year ago

Thanks @jfversluis. I searched everywhere for that url, but I wasn't able to find it, even it was right under my nose

nicholaslambertucci-ett commented 1 year ago

Tested with version 1.0.0-build-1002.87588 and DiconnectHander doesn't throw exception anymore.

jfversluis commented 1 year ago

Perfect, thanks!

zisi911 commented 1 year ago

Hello guys,

I have the latest .net 7 update and the latest communiltytoolkit and mediaelements installed (currently 5.1.0 and 1.0.2) and this issue is there. As soon as i dispose the handler and it tries to go to the previous view or any other i get the crash (unhandled expection). any ideas?

pictos commented 1 year ago

@zisi911 please open a new issue and provide a small sample that reproduces the issue