Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug: Null Reference Exceptions in SetAsyncMethodContinuation() #3402

Open
RickStrahl opened this issue Apr 18, 2023 · 10 comments
Open

Bug: Null Reference Exceptions in SetAsyncMethodContinuation() #3402

RickStrahl opened this issue Apr 18, 2023 · 10 comments
Assignees
Labels
bug Something isn't working priority-low We have considered this issue and decided that we will not be able to address it in the near future. tracked We are tracking this work internally.

Comments

@RickStrahl
Copy link

RickStrahl commented Apr 18, 2023

I'm running into a null reference exception in CoreWebView2PrivateHostObjectHelper. In the debugger I can see where the exception originates:

image

The stack trace doesn't originate in my code but the code I'm launching from has to do with WebView key handling which no longer works and appears to have been broken. The stacktrace in the debugger shows the main event loop as the source which is likely due to a Dispatcher originated operation in my code.

In my logs, however the error shows up as:

4/17/2023 6:22:49 PM - AppRoot: Attempted to read or write protected memory. This is often an indication that other memory is corrupt.
Attempted to read or write protected memory. This is often an indication that other memory is corrupt.
Markdown Monster v2.8.15.1
10.0.22621.1.amd64fre.ni_release.220506-1250 - en-US - NET 4.8 - 64 bit
Dell Inc. XPS 15 9520, NVIDIA GeForce RTX 3050 Laptop GPU, hw-acc: True
en-US - en-US    
---
WindowsBase
   at MS.Win32.UnsafeNativeMethods.DispatchMessage(MSG& msg)
   at System.Windows.Threading.Dispatcher.PushFrameImpl(DispatcherFrame frame)
   at System.Windows.Application.RunDispatcher(Object ignore)
   at System.Windows.Application.RunInternal(Window window)
   at StartUp.Main() in D:\projects\MarkdownMonsterCode\MarkdownMonster\Program.cs:line 53
System.AccessViolationException
---------------------------

I'm seeing a ton of these in my application and I suspect they are all hitting this same issue.

AB#44249026

@RickStrahl RickStrahl added the bug Something isn't working label Apr 18, 2023
@RickStrahl RickStrahl changed the title Bug: Null Reference Exceptions Bug: Null Reference Exceptions in SetAsyncMethodContinuation() Apr 18, 2023
@RickStrahl
Copy link
Author

RickStrahl commented Apr 18, 2023

After a little more exploration in my code this is due to my Alt-Menu key forwarding code that has been working but is now failing. It looks like the above method only gets triggered when making a sync method call, but avoided on an async call.

I ended up changing the call on client side from sync JavaScript to Async and that now doesn't hit that code path which fixes the immediate issue to me.

// check for Windows Alt-Menu behavior
// delay trigger menu so native alt- key processing can work
if (te.mm && event.key == "Alt" && !event.ctrlKey && !event.shiftKey) {
    te.lastAltkeyTime = new Date().getTime();
    
    setTimeout(async function () {
        console.log("alt key triggered 2 (timeout)");
        var t = new Date().getTime();
        //console.log('alt timeout: ' + (t - te.lastAltkeyTime), t, te.lastAltkeyTime);
        // other key or more than 5 secs
        if (te.lastAltkeyTime == 0 || t - te.lastAltkeyTime > 5000) {
            te.lastAltkeyTime = 0;
            return;
        }                        
        // await here avoids the exception here compared to sync call used previously
        await te.mmAsync.TriggerWindowAltMenu();                        
    }, 500);
} else
    te.lastAltkeyTime = 0;

I'm curious to see if this fixes the persistent Attempted to read or write protected memory. errors I've been seeing in my logs.

Regardless of my fix though, the issue still remains and appears to be changed behavior from previous releases as I did not see these errors until about a month ago or so.

There's some problem in the above outlined method where apparently the incoming type is null and should probably be checked for.

ps.
The ultimate solution to this whole thing would be for the WebView to pass forward Alt key combos for menu activations like the WebBrowser did as described in issue #468

@novac42
Copy link
Contributor

novac42 commented Apr 19, 2023

Thanks for reaching out. I've assigned this to a dev that can help follow up on this.

@david-risney david-risney added the tracked We are tracking this work internally. label Apr 19, 2023
@david-risney
Copy link
Contributor

Thanks! Looks like a regression we'll need to fix. I've marked it tracked to get it in our bug database.

@RickStrahl
Copy link
Author

RickStrahl commented Apr 19, 2023

Just as a follow up - I made the JS Async method fix I described above that avoids a drive-by of this method. As soon as I made this change I stopped getting the

Attempted to read or write protected memory.

errors in my log. I've moved to production and haven't seen any of those errors in the last 2.5 days so pretty sure that was the problem.

The old code had been in place for years and there was never a problem, so something definitely changed about a month or two ago when a lot of these errors started showing up in my logs. It might help pinpoint when a change was made in that code.

This seems to suggest that the erroring method gets triggered by synthetic async code - my term for async code that hasn't been properly initialized by either WPF or by a callback from the WebView. In this specific case (key forwarding from JS->.NET) the sync call from JS->.NET along with a .FireAndForget() call of an async method triggered the drive by. Changing the incoming JS code to async was the fix for this specific scenario.

@lostobject
Copy link

I am also seeing this exception in a different scenario. In my case I am calling async methods on the HostObject (updating my app to use #75) and trying to propagate exceptions back to javascript. This works fine for synchronous HostObject methods but async ones trigger the NullReferenceException on the same line of code as posted above.

WebView2 version: 1.0.2151.40

Here is the relevant code from the HostObject:

        public string TestMessage(string msg)
        {
            throw new ArgumentException();
        }

        public async Task<string> TestMessageAsync(string msg)
        {
            await Task.Run(() => throw new ArgumentException());
            return msg;
        }

And the test html page:

    async function Test1() {
        try {
          let external = await window.chrome.webview.hostObjects.external;
          let data = await external.TestMessage("test sync call);
          document.getElementById("result-TestMessage").value = data;
        } catch (e) {
          document.getElementById("result-TestMessage").value = `Error: ${e.toString()}`;
        }
    }

    async function Test2() {
        try {
          let external = await window.chrome.webview.hostObjects.external;
          let data = await external.TestMessageAsync("test async call);
          document.getElementById("result-TestMessage").value = data;
        } catch (e) {
          document.getElementById("result-TestMessage").value = `Error: ${e.toString()}`;
        }
    }

Running Test1() renders the expected output:
Error: Error: Value does not fall within the expected range.

Running Test2() results in the internal NullReferenceException and the application crashes:
Microsoft.Web.WebView2.Core.dll!Microsoft.Web.WebView2.Core.CoreWebView2PrivateHostObjectHelper.RawHelper.SetAsyncMethodContinuation.AnonymousMethod__0() Line 100 C#

@NiklasPor
Copy link

@lostobject did you manage to work out a workaround? I'm running into the exact same issue, oterwise I'll need to wrap every of my tasks for error handling 👁️

@lostobject
Copy link

@NiklasPor No I have not found any workarounds other than 'make sure exceptions never get thrown'. This is not a valid workaround for me since I need to propagate errors back to javascript.

@david-risney Has there been any progress on this issue from your side?

@3ldor
Copy link

3ldor commented Feb 7, 2024

Ran into the same issue when throwing exceptions, would be nice to have this fixed.
Running WebView2 1.0.2277.86

@github-actions github-actions bot added the priority-low We have considered this issue and decided that we will not be able to address it in the near future. label Mar 26, 2024
@LiangTheDev
Copy link
Member

Changes for #4509 might have changed the behavior. Is there any repro after updating WebView2 SDK to version > 1.0.2562?

@3ldor
Copy link

3ldor commented Sep 19, 2024

Changes for #4509 might have changed the behavior. Is there any repro after updating WebView2 SDK to version > 1.0.2562?

Seems like the browser no longer crashes, and a JavaScript exception is triggered as expected. The exception doesn't seem to contain any of the originating information however, the message always seems to be Error (0x13D) while retrieving error. (0x80131604), so I wouldn't consider this fixed.

Tested on 1.0.2739.15 on .NET 8.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority-low We have considered this issue and decided that we will not be able to address it in the near future. tracked We are tracking this work internally.
Projects
None yet
Development

No branches or pull requests

7 participants