CEF crashes

[aligre]

Hello
We face many crashes during the initilization of CEF but it only happens at customer side, not at R&D, so it's difficult to debug them.
Involved CEF version can be 142 or 144, depending on the customer.
OS is windows
Particular configuration : OSR mode + multi_threaded_message_loop==true
We have captured (minidump) the callstack for the crash, in the CEFUIThread :

Code: Select all

[Inline Frame] std::__Cr::unique_ptr<...>::reset Line 285   C++
[Inline Frame] std::__Cr::unique_ptr<...>::operator= Line 225   C++
ChromeContentBrowserClient::SetSamplingProfiler Line 8764   C++
[Inline Frame] CefMainRunner::BeforeUIThreadInitialize Line 453   C++
[Inline Frame] CefMainRunner::ContentMainRun::<lambda> Line 311   C++
[Inline Frame] base::internal::DecayedFunctorTraits<...>::Invoke Line 648   C++
[Inline Frame] base::internal::InvokeHelper<...>::MakeItSo Line 922   C++
[Inline Frame] base::internal::Invoker<...>::RunImpl Line 1059   C++
base::internal::Invoker<...>::RunOnce Line 972   C++
[Inline Frame] base::OnceCallback<...>::Run Line 155   C++
CefUIThread::ThreadMain Line 98   C++
base::`anonymous namespace'::ThreadFunc Line 105   C++


This task has been initiated in the main thread method CefMainRunner::ContentMainRun with the call

Code: Select all

if (multi_threaded_message_loop_) {
    ...
    if (!CreateUIThread(base::BindOnce(
            [](CefMainRunner* runner, base::WaitableEvent* event,
               int* exit_code) {
              runner->BeforeUIThreadInitialize();
   ...
}


and here below the code of method BeforeUIThreadInitialize

Code: Select all

void CefMainRunner::BeforeUIThreadInitialize() {
  static_cast<ChromeContentBrowserClient*>(
      CefAppManager::Get()->GetContentClient()->browser())
      ->SetSamplingProfiler(
          std::make_unique<MainThreadStackSamplingProfiler>());


I guess the method GetContentClient()->browser() returns nullptr but after having debugged a normal scenario (i.e. that not crashes), I didn't succeed to imagine conditions where this method returns a nullptr.

any idea is welcome ...

[aligre]

I have an hypothesis that can explain the null pointer object access in our crashes :

when the CEFUIThread is created

Code: Select all

        CefUIThread::Start Line 40
>   CefMainRunner::CreateUIThread Line 405
    CefMainRunner::ContentMainRun Line 320
    CefMainRunner::Initialize Line 72
    CefContext::Initialize Line 399
    CefInitialize Line 232


the object

ChromeMainDelegateCef -> chrome_content_client_cef_ -> browser_

has already been created during following initialization step

Code: Select all

   content::ContentClientInitializer::Set Line 522   C++
    content::ContentMainRunnerImpl::Initialize Line 922   C++
    content::ContentMainInitialize Line 318   C++
    CefMainRunner::ContentMainInitialize Line 284   C++
    CefMainRunner::Initialize Line 59   C++
    CefContext::Initialize Line 399   C++
    CefInitialize Line 232   C++


I suppose that's why the code doesn't test if it exists before calling on it the method SetSamplingProfiler in CEFUIThread

Code: Select all

void CefMainRunner::BeforeUIThreadInitialize() {
  static_cast<ChromeContentBrowserClient*>(
      CefAppManager::Get()->GetContentClient()->browser())
      ->SetSamplingProfiler(
          std::make_unique<MainThreadStackSamplingProfiler>());
}


But looking at CefUIThread::Start, I have the intuition that this thread has already started in the chromium base thread infrastructure.
CefUIThread::Start would be not a thread creation/start but more the association of a task to an already running thread.
If this intuition is right, and because I haven't seen synchronism mechanism (mutex?) to ensure the memory visibility of browser_ in CEFUIThread, maybe the many crashes we have are caused by that.
In CefUIThread, browser_ still contains its old value, nullptr, then the call to SetSamplingProfiler crashes.

[ndesktop]

I second that. Without the multithreaded flag, the sequence looks more serialized (I'm saying "more" because I don't know if the calls are direct or there are tasks posted to the loop); but with the flag set, maybe there should be some kind of callback invoking BeforeUIThreadInitialize when the browser_ creation is done.

[aligre]

From a debug session of a normal scenario (i.e. not a crash) , I've seen the browser_ creation is always done before the call to CefMainRunner::CreateUIThread
My point is more that CreateUIThread not creates a thread but get an already existing thread, and I havent seen any memory barrier when accessing to browser_ in the CEFUIThread.
Then it's possible that CEFUIThread access to an old thread cache value and not the real one
It's maybe needed to add a memory barrier when accessing to that pointer

[aligre]

If this memory barrier issue is real, a simple way to fix that can be to add a parameter browser to the method BeforeUIThreadInitialize.
Something like

Code: Select all

void CefMainRunner::BeforeUIThreadInitialize(ChromeContentBrowserClient* iBrowser)
{
  if (iBrowser)
      iBrowser->SetSamplingProfiler(
          std::make_unique<MainThreadStackSamplingProfiler>());
}

and in the caller method CefMainRunner::ContentMainRun

Code: Select all

  auto * browser= static_cast<ChromeContentBrowserClient*>(
      CefAppManager::Get()->GetContentClient()->browser());
  if (multi_threaded_message_loop_) {
     ...
     if (!CreateUIThread(base::BindOnce(
            [](CefMainRunner* runner, base::WaitableEvent* event,
               int* exit_code, ChromeContentBrowserClient * browser) {
              runner->BeforeUIThreadInitialize(browser);
             ...
  } else {
    ...
    BeforeUIThreadInitialize(browser);


With that, the method BeforeUIThreadInitialize will work with the uptodate value of browser_

[aligre]

My previous sentence CreateUIThread not creates a thread but get an already existing thread, is not right.
After debugging deeply in base\threading\platform_thread_win.cc it seems to me that CreateUIThread actually creates a new thread thanks to a call to MS api ::CreateThread.
However, this API offers no guarantees regarding memory synchronization; the hypothesis may therefore still hold: CEFUIThread is not reading the correct value of `browser_` because memory synchronization is lacking during that read operation.