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

NullException with Async JS calls #493

Closed
Bodekaer opened this issue Sep 26, 2014 · 17 comments
Closed

NullException with Async JS calls #493

Bodekaer opened this issue Sep 26, 2014 · 17 comments
Labels
Milestone

Comments

@Bodekaer
Copy link
Contributor

Hey @amaitland,

I sometimes get a null exception on this line:
https://github.com/cefsharp/CefSharp/blob/JsBinding_WIP/CefSharp.Core/ManagedCefBrowserAdapter.h#L433

I cannot easily replicate it, but perhaps you can easily see what it could be?

@amaitland
Copy link
Member

@Bodekaer I'll update the method to at least gracefully handle the null, unsure first off about what could be causing it, I can clean it up a little. Does it happen frequently enough to know if it's been successfully fixed?

Soon as #492 is merged into master I can update the JS branch, the logging API has been exposed, so we can put in some logging deeper into the Cef side of things and hopefully see what's going on.

@amaitland amaitland added the cef3 label Sep 28, 2014
@amaitland amaitland added this to the 33.0.0 milestone Sep 28, 2014
@Bodekaer
Copy link
Contributor Author

Sounds good. Stability is most important for us at the moment, and if the null is just handled for now, that should be ok.
It happens usually every 2-3 days, so I'll be able to report after 1 week whether it's still occurring with a pretty good certainty.

amaitland added a commit to amaitland/CefSharp that referenced this issue Sep 29, 2014
…s occurring as per cefsharp#493.

Cleanup logic to handle null pointer, also simplify as was a little convoluted.
@amaitland
Copy link
Member

I've simplified the method, and handled the possibility of null for frame

Let me know what you think

amaitland@16f3bc4

@Bodekaer
Copy link
Contributor Author

Looks good.
Then I can always handle the null that is returned.

Thanks.

@Bodekaer
Copy link
Contributor Author

How about including error info in the JavascriptResponse perhaps?
In case frame is null, I wouldn't really know what happened.

E.g. I would need to write something like:
if (cefBrowser.EvaluateScriptAsync(...) == null) { handleError(null); };
Perhaps instead return an empty JavascriptResponse with error info.

@amaitland
Copy link
Member

Sounds like a reasonable request.

@Bodekaer
Copy link
Contributor Author

I've upgraded to the latest updates from your branch, and now I get a similar null exception, but in this line of code:
https://github.com/amaitland/CefSharp/blob/JsBinding/Wip2/CefSharp.Core/ManagedCefBrowserAdapter.h#L115

Here is the call stack:

CefSharp.Core.dll!.CefRefPtr.{ctor}(CefRefPtr* r) Line 157 C++
CefSharp.Core.dll!.CefSharp.Internals.ClientAdapter.GetCefBrowser() Line 56 C++
CefSharp.Core.dll!CefSharp.ManagedCefBrowserAdapter.OnInitialized() Line 115 C++
CefSharp.Core.dll!.CefSharp.Internals.ClientAdapter.OnAfterCreated() Line 47 C++

@Bodekaer
Copy link
Contributor Author

It still only happens in rare cases, and so far I'm not able to reproduce it.
I believe it's related to disposing the browser - although in this case it happens during initialization which is strange.

@amaitland
Copy link
Member

Strange!

I've pushed a couple more changes, most are cleanup as I'm going, relevant one to you should be this one
amaitland@6b542ba

Basically passing in BrowserId as a param, should have a valid reference to browser.

See how that goes?

@Bodekaer
Copy link
Contributor Author

Bodekaer commented Oct 1, 2014

Cool thx. I'll test it out.

@amaitland
Copy link
Member

@Bodekaer Any luck with the latest set of changes?

@Bodekaer
Copy link
Contributor Author

Bodekaer commented Oct 3, 2014

Sorry, been busy last few days with launching this years PG event (http://www.projectgetaway.com/).
I'll be fully back to coding on Sunday, and test it all out there.

@amaitland
Copy link
Member

No problem!

FYI: As per #496 (comment) we're going to release a stable version 33.0.0 without JS Binding and release 33.1.0-pre version including the JS Binding shortly after.

So just looking at any open issues that maybe relevant to this new -pre release.

@Bodekaer
Copy link
Contributor Author

Bodekaer commented Oct 3, 2014

Ahh cool. It's ok. We're anyways compiling our own builds here for now (with smaller additions here and there)

@amaitland
Copy link
Member

@Bodekaer Still experiencing problems?

@Bodekaer
Copy link
Contributor Author

Nope. All good here. Thanks for the heads up.

@amaitland
Copy link
Member

Cool cool, no problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants