PopoverService: Fix DisposeAsync causing hangs on MAUI

[Mr-Technician]

Description

In Blazor hybrid environments, awaiting a JS interop call may cause the app to hang, such as in: dotnet/maui#7997

I have investigated in this repo and this PR resulted after discussing with @henon.

However, the unit test MudPopoverService_DisposeAsync_ThrowsExceptionIfNotTaskCancelException needs some work, so if @just-the-benno could take a look, that would be great.

Resolves #6628.

How Has This Been Tested?

Unit tests still need work

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist:

• The PR is submitted to the correct branch (dev).
• My code follows the code style of this project.
• I've added relevant tests.

[Mr-Technician]

Looping in @mikes-gh.

@Eilon tagging you in case you have anything to add.

[MackinnonBuck]

Broadly speaking, JS interop being performed outside of a Blazor context is not a supported scenario for Blazor Hybrid.

However, the bug here appears to be something bigger. Since Windows Forms controls must be disposed synchronously, Blazor Hybrid synchronously blocks on the task disposing the service scope. However, this ends up causing a deadlock when a service actually does something asynchronous on disposal (like JS interop). I was able to reproduce a similar hanging bug by reistering a dummy service that implements IAsyncDisposable and awaits Task.Yield().

I'm going to start a discussion on the original thread in dotnet/maui#7997 about potential fixes for this.

[just-the-benno]

Despite the described issue, the reason I implemented IAsyncDisposable in this service was to be able to call a JS method that cleans up the used array and resize and mutation observer. Since Blazor has support for IAsyncDisposable, I thought it is cleaner than relying on AndForget() and implementing it in a "normal" Dispose(). And yes, it makes testing also easier ;)

That said, the functional difference between Dispose() and DisposeAsync() is marginal in this scenario. If you want a way forward, I'd comment out the relevant IAsyncDisposable parts (and tests) and implement an IDisposable version of it.

Hopefully, through the started discussion dotnet/maui#7997 we will know if IAsyncDisposable is even a recommended scenario for Blazor after all.

[mikes-gh]

@just-the-benno @Mr-Technician Looks like framework fix wont come until net8.0
We need to decide how to avoid the deadlock until then. I don't like it but I am guessing we don't have much choice other than to use Synchronous dispose. What do we lose. Testing and error trapping?

[henon]

Maybe there is a way to detect that we are running as Blazor Hybrid and just in that case refrain from awaiting?

[Mr-Technician]

I like the idea @henon, but it might be hard to detect given that hybrid apps (at least the winforms ones) run under WebView, which is essentially Microsoft Edge under the hood. I'm not sure we can tell if a "real" browser is involved or not.

[henon]

Yeah. To be honest, I don't see a problem in either not waiting (.AndForget()) or switching to sync dispose. I'd even vote for the first. But let's hear @just-the-benno 's opinion.

[mikes-gh]

Maybe the Navigator in blazor gives info?

[Mr-Technician]

@mikes-gh @NavigationManager.Uri and @NavigationManager.BaseUri both return https://0.0.0.0/ but that's not something we can go off of.

[cinderbear]

Maybe you can leave this choice to the developer, with a parameter like "IsBlazorHybrid"
(it would be similar to using the parameter "IsDarkMode" in MudThemeProvider).

[henon]

@cinderbear Sure, if you can give a good reason why.

[Mr-Technician]

@henon The reason would be to alert MudBlazor of a hybrid environment to avoid awaiting a JS disposal indefinitely: #5367 (comment)

However, handling the root issue of .AndForget() is probably best.

[cinderbear]

Since this issue seems caused by subtle differences in the behavior of webview depending on the underliying platform (like this case that appears to affect winforms) it may be useful to have a parameter to differentiate this kind of usage, given that it's very difficult or impossible to detect automatically.

But if you consider that the fix would not affect any other use cases, it may not be currently needed.

[mikes-gh]

@Mr-Technician how about UA
What does this give on your webview

https://stackoverflow.com/questions/37591279/detect-if-user-is-using-webview-for-android-ios-or-a-regular-browser

var standalone = window.navigator.standalone,
  userAgent = window.navigator.userAgent.toLowerCase(),
  safari = /safari/.test(userAgent),
  ios = /iphone|ipod|ipad/.test(userAgent);

if (ios) {
  if (!standalone && safari) {
    // Safari
  } else if (!standalone && !safari) {
    // iOS webview
  };
} else {
  if (userAgent.includes('wv')) {
    // Android webview
  } else {
    // Chrome
  }
};

[Mr-Technician]

@mikes-gh Good lead, but unfortunately the UA isn't distinguishable from a normal browser:

Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/107.0.0.0 Safari/537.36 Edg/107.0.1418.52