OnProtocolExecution: crash when using CefBrowser->StopLoad() with allow_os_execution = true

[HashidaTKS]

Describe the bug

This is similar to #3821.

My application implements the OnProtocolExecution handler like below.

void ClientHandler::OnProtocolExecution(CefRefPtr<CefBrowser> browser, CefRefPtr<CefFrame> frame, CefRefPtr<CefRequest> request, bool& allow_os_execution)
{
	allow_os_execution = true;
	browser->StopLoad();
}

This implementation crashes when opening zoom addresses.
This implementation works fine on CEF 127, but crashes on CEF128+.

This implementation crashes even on 131.2.7+g9a14dc9+chromium-131.0.6778.86, while it seems that the fix for #3821, I mean 0860ec2, is already applied to that version.

To Reproduce

• Implement OnProtocolExecution like Describe the bug.
• Open a zoom address.

Expected behavior

A pop-up asking if we want to open the installed zoom appears.

Screenshots
If applicable, add screenshots to help explain your problem.

Versions (please complete the following information):

• OS: Windows11 24H2
• CEF Version: 131.2.7+g9a14dc9+chromium-131.0.6778.86

Additional context

• Does the problem reproduce with the cefclient or cefsimple sample application at the same version?

  • No, I guess they don't use browser->StopLoad(); and allow_os_execution = true; in OnProtocolExecution.

• Does the problem reproduce with Google Chrome at the same version?

  • No

• Add any other context about the problem here.

Here is a stack trace when crashing.

 	libcef.dll!logging::LogMessage::HandleFatal(unsigned int stack_start, const std::__Cr::basic_string<char,std::__Cr::char_traits<char>,std::__Cr::allocator<char>> & str_newline) Line 1050	C++
 	[Inline Frame] libcef.dll!logging::LogMessage::Flush::<lambda_0>::operator()() Line 750	C++
 	[Inline Frame] libcef.dll!absl::cleanup_internal::Storage<`lambda at ..\..\base\logging.cc:748:40'>::InvokeCallback() Line 87	C++
 	[Inline Frame] libcef.dll!absl::Cleanup<absl::cleanup_internal::Tag,`lambda at ..\..\base\logging.cc:748:40'>::~Cleanup() Line 106	C++
 	libcef.dll!logging::LogMessage::Flush() Line 933	C++
 	libcef.dll!logging::LogMessageFatal::~LogMessageFatal() Line 1056	C++
 	libcef.dll!base::allocator::UnretainedDanglingRawPtrDetectedCrash(unsigned int id) Line 751	C++
 	[Inline Frame] libcef.dll!partition_alloc::internal::InSlotMetadata::ReportIfDangling() Line 311	C++
 	libcef.dll!base::internal::RawPtrBackupRefImpl<1,0>::ReportIfDanglingInternal(unsigned int address) Line 70	C++
 	[Inline Frame] libcef.dll!base::internal::RawPtrBackupRefImpl<1,0>::ReportIfDangling(content::NavigationUIData * wrapped_ptr) Line 430	C++
 	[Inline Frame] libcef.dll!base::raw_ptr<content::NavigationUIData,1>::ReportIfDangling() Line 1023	C++
 	[Inline Frame] libcef.dll!base::internal::UnretainedWrapper<content::NavigationUIData,base::unretained_traits::MayNotDangle,0>::GetInternal(const base::raw_ptr<content::NavigationUIData,1> & ptr) Line 172	C++
 	[Inline Frame] libcef.dll!base::internal::UnretainedWrapper<content::NavigationUIData,base::unretained_traits::MayNotDangle,0>::get() Line 154	C++
 	[Inline Frame] libcef.dll!base::BindUnwrapTraits<base::internal::UnretainedWrapper<content::NavigationUIData,base::unretained_traits::MayNotDangle,0>>::Unwrap(const base::internal::UnretainedWrapper<content::NavigationUIData,base::unretained_traits::MayNotDangle,0> & o) Line 1953	C++
 	[Inline Frame] libcef.dll!base::internal::Unwrap(const base::internal::UnretainedWrapper<content::NavigationUIData,base::unretained_traits::MayNotDangle,0> & o) Line 435	C++
 	[Inline Frame] libcef.dll!base::internal::InvokeHelper<0,base::internal::FunctorTraits<void (*const &)(ChromeContentBrowserClientCef *, base::RepeatingCallback<content::WebContents *()>, base::IdType<content::FrameTreeNodeIdTag,int,-1,1,0>, content::NavigationUIData *, bool, bool, network::mojom::WebSandboxFlags, const network::ResourceRequest &, const std::__Cr::optional<url::Origin> &, content::WeakDocumentPtr, const net::IsolationInfo &),ChromeContentBrowserClientCef *,const base::RepeatingCallback<content::WebContents *()> &,const base::IdType<content::FrameTreeNodeIdTag,int,-1,1,0> &,content::NavigationUIData *const &,const bool &,const bool &,const network::mojom::WebSandboxFlags &,const network::ResourceRequest &,const std::__Cr::optional<url::Origin> &,const content::WeakDocumentPtr &,const net::IsolationInfo &>,void,0,1,2,3,4,5,6,7,8,9,10>::MakeItSo(void(*)(ChromeContentBrowserClientCef *, base::RepeatingCallback<content::WebContents *()>, base::IdType<content::FrameTreeNodeIdTag,int,-1,1,0>, content::NavigationUIData *, bool, bool, network::mojom::WebSandboxFlags, const network::ResourceRequest &, const std::__Cr::optional<url::Origin> &, content::WeakDocumentPtr, const net::IsolationInfo &) & functor, const std::__Cr::tuple<base::internal::UnretainedWrapper<ChromeContentBrowserClientCef,base::unretained_traits::MayNotDangle,0>,base::RepeatingCallback<content::WebContents *()>,base::IdType<content::FrameTreeNodeIdTag,int,-1,1,0>,base::internal::UnretainedWrapper<content::NavigationUIData,base::unretained_traits::MayNotDangle,0>,bool,bool,network::mojom::WebSandboxFlags,network::ResourceRequest,std::__Cr::optional<url::Origin>,content::WeakDocumentPtr,net::IsolationInfo> & bound) Line 930	C++
 	[Inline Frame] libcef.dll!base::internal::Invoker<base::internal::FunctorTraits<void (*const &)(ChromeContentBrowserClientCef *, base::RepeatingCallback<content::WebContents *()>, base::IdType<content::FrameTreeNodeIdTag,int,-1,1,0>, content::NavigationUIData *, bool, bool, network::mojom::WebSandboxFlags, const network::ResourceRequest &, const std::__Cr::optional<url::Origin> &, content::WeakDocumentPtr, const net::IsolationInfo &),ChromeContentBrowserClientCef *,const base::RepeatingCallback<content::WebContents *()> &,const base::IdType<content::FrameTreeNodeIdTag,int,-1,1,0> &,content::NavigationUIData *const &,const bool &,const bool &,const network::mojom::WebSandboxFlags &,const network::ResourceRequest &,const std::__Cr::optional<url::Origin> &,const content::WeakDocumentPtr &,const net::IsolationInfo &>,base::internal::BindState<0,1,0,void (*)(ChromeContentBrowserClientCef *, base::RepeatingCallback<content::WebContents *()>, base::IdType<content::FrameTreeNodeIdTag,int,-1,1,0>, content::NavigationUIData *, bool, bool, network::mojom::WebSandboxFlags, const network::ResourceRequest &, const std::__Cr::optional<url::Origin> &, content::WeakDocumentPtr, const net::IsolationInfo &),base::internal::UnretainedWrapper<ChromeContentBrowserClientCef,base::unretained_traits::MayNotDangle,0>,base::RepeatingCallback<content::WebContents *()>,base::IdType<content::FrameTreeNodeIdTag,int,-1,1,0>,base::internal::UnretainedWrapper<content::NavigationUIData,base::unretained_traits::MayNotDangle,0>,bool,bool,network::mojom::WebSandboxFlags,network::ResourceRequest,std::__Cr::optional<url::Origin>,content::WeakDocumentPtr,net::IsolationInfo>,void ()>::RunImpl(void(*)(ChromeContentBrowserClientCef *, base::RepeatingCallback<content::WebContents *()>, base::IdType<content::FrameTreeNodeIdTag,int,-1,1,0>, content::NavigationUIData *, bool, bool, network::mojom::WebSandboxFlags, const network::ResourceRequest &, const std::__Cr::optional<url::Origin> &, content::WeakDocumentPtr, const net::IsolationInfo &) & functor, const std::__Cr::tuple<base::internal::UnretainedWrapper<ChromeContentBrowserClientCef,base::unretained_traits::MayNotDangle,0>,base::RepeatingCallback<content::WebContents *()>,base::IdType<content::FrameTreeNodeIdTag,int,-1,1,0>,base::internal::UnretainedWrapper<content::NavigationUIData,base::unretained_traits::MayNotDangle,0>,bool,bool,network::mojom::WebSandboxFlags,network::ResourceRequest,std::__Cr::optional<url::Origin>,content::WeakDocumentPtr,net::IsolationInfo> & bound, std::__Cr::integer_sequence<unsigned int,0,1,2,3,4,5,6,7,8,9,10>) Line 1067	C++
 	libcef.dll!base::internal::Invoker<base::internal::FunctorTraits<void (*const &)(ChromeContentBrowserClientCef *, base::RepeatingCallback<content::WebContents *()>, base::IdType<content::FrameTreeNodeIdTag,int,-1,1,0>, content::NavigationUIData *, bool, bool, network::mojom::WebSandboxFlags, const network::ResourceRequest &, const std::__Cr::optional<url::Origin> &, content::WeakDocumentPtr, const net::IsolationInfo &),ChromeContentBrowserClientCef *,const base::RepeatingCallback<content::WebContents *()> &,const base::IdType<content::FrameTreeNodeIdTag,int,-1,1,0> &,content::NavigationUIData *const &,const bool &,const bool &,const network::mojom::WebSandboxFlags &,const network::ResourceRequest &,const std::__Cr::optional<url::Origin> &,const content::WeakDocumentPtr &,const net::IsolationInfo &>,base::internal::BindState<0,1,0,void (*)(ChromeContentBrowserClientCef *, base::RepeatingCallback<content::WebContents *()>, base::IdType<content::FrameTreeNodeIdTag,int,-1,1,0>, content::NavigationUIData *, bool, bool, network::mojom::WebSandboxFlags, const network::ResourceRequest &, const std::__Cr::optional<url::Origin> &, content::WeakDocumentPtr, const net::IsolationInfo &),base::internal::UnretainedWrapper<ChromeContentBrowserClientCef,base::unretained_traits::MayNotDangle,0>,base::RepeatingCallback<content::WebContents *()>,base::IdType<content::FrameTreeNodeIdTag,int,-1,1,0>,base::internal::UnretainedWrapper<content::NavigationUIData,base::unretained_traits::MayNotDangle,0>,bool,bool,network::mojom::WebSandboxFlags,network::ResourceRequest,std::__Cr::optional<url::Origin>,content::WeakDocumentPtr,net::IsolationInfo>,void ()>::Run(base::internal::BindStateBase * base) Line 987	C++
 	[Inline Frame] libcef.dll!base::OnceCallback<void ()>::Run() Line 156	C++
 	libcef.dll!base::TaskAnnotator::RunTaskImpl(base::PendingTask & pending_task) Line 202	C++
 	[Inline Frame] libcef.dll!base::TaskAnnotator::RunTask(perfetto::StaticString event_name, base::PendingTask & pending_task, base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWorkImpl::<lambda_4> && args) Line 98	C++
 	[Inline Frame] libcef.dll!base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWorkImpl(base::LazyNow * continuation_lazy_now) Line 471	C++
 	libcef.dll!base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::DoWork() Line 332	C++
 	[Inline Frame] libcef.dll!`anonymous namespace'::MessagePumpExternal::DirectRunWork(base::MessagePump::Delegate * delegate, base::TimeTicks * next_run_time) Line 68	C++
 	libcef.dll!`anonymous namespace'::MessagePumpExternal::Run(base::MessagePump::Delegate * delegate) Line 37	C++
 	libcef.dll!base::sequence_manager::internal::ThreadControllerWithMessagePumpImpl::Run(bool application_tasks_allowed, base::TimeDelta timeout) Line 641	C++
 	libcef.dll!base::RunLoop::Run(const base::Location & location) Line 135	C++
 	libcef.dll!base::RunLoop::RunUntilIdle() Line 144	C++
 	libcef.dll!CefDoMessageLoopWork() Line 382	C++

[leediesel]

I also confront the same issue , even I apply the patch :#3821

[mbragg12]

Similar. The verbose logging seems a little more useful

[66776:63688:1213/154815.934:FATAL:partition_alloc_support.cc(751)] Detected dangling raw_ptr in unretained with id=0x00005c1c04f411ac:
Task trace:
	net_service::`anonymous namespace'::InterceptedRequestHandlerWrapper::OnRequestComplete [0x00007FFD54FB9466+838] (C:\cef\code\chromium_git\chromium\src\cef\libcef\browser\net_service\resource_request_handler_wrapper.cc:1119)
	mojo::SimpleWatcher::Context::CallNotify [0x00007FFD58255E84+260] (C:\cef\code\chromium_git\chromium\src\mojo\public\cpp\system\simple_watcher.cc:61)
	CefBrowserInfo::NavigationLock::~NavigationLock [0x00007FFD54F55604+116] (C:\cef\code\chromium_git\chromium\src\cef\libcef\browser\browser_info.cc:410)
	base::internal::Invoker<base::internal::FunctorTraits<`lambda at ..\..\cef\libcef\browser\browser_host_create.cc:92:7' &&,std::__Cr::unique_ptr<(anonymous namespace)::CreateBrowserHelper,std::__Cr::default_delete<(anonymous namespace)::CreateBrowserHelper [0x00007FFD54F52A9C+156] (C:\cef\code\chromium_git\chromium\src\base\functional\bind_internal.h:980)
	CefRequestContextImpl::ExecuteWhenBrowserContextInitialized [0x00007FFD54FDD276+214] (C:\cef\code\chromium_git\chromium\src\cef\libcef\browser\request_context_impl.cc:253)

[mbragg12]

It appears to be a threading issue. The internal callback is getting yanked out from under the handler. Basically, it works if the OnProtocolExecution call returns semi immediately. But if there is a delay in the response it crashes and burns.
I can reproduce the issue in the cefclient by adding a one second sleep to the OnProtocolExecution function.

@magreenblatt I assume it is the init_state_->unhandled_request_callback_ that is being released based on the logging. Any suggestions on how to keep that alive until OnProtocolExecution responds?

[mbragg12]

Don't know if it helps in any way but I am assuming these protocol issues were introduced by the changes associated with this https://docs.google.com/document/d/1LjxHl32fE4tCKugrK_PIso7mfXQVEeoD1wSnX2y0ZU8/edit?usp=sharing&resourcekey=0-d1gP4X2sG7GPl9mlTeptIA
There are a few Chrome defects related this change as well.

[magreenblatt]

From the original call stack this looks like a lifespan issue with the NavigationUIData* argument bound in the HandleExternalProtocolHelper callback (lifespan controlled by NavigationURLLoaderImpl). We could potentially use NavigationUIData::Clone() instead and pass ownership of the cloned object to the binding.

[mbragg12]

@magreenblatt This sounds like a plan to me, but I can't say I know the code well enough to understand all the repercussions of that.
It would be great to try something though. We on rare occasion still see the crash.
If I can help, let me know.

[magreenblatt]

Please test in M135 (builds to be available tomorrow) and report any issues.

[HashidaTKS]

I tested with the reproduction steps in M135 and it worked fine. Thanks!

[mbragg12]

Same. Testing using a tweaked cefclient and appears to work with a delay again. Thanks.