bcarrella
/
cef
mirror of https://bitbucket.org/chromiumembedded/cef
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Don't use NotificationStateLock for GetMainFrame (see issue #2421) 

This causes a race related to |notification_state_lock_| assignment when
GetMainFrame is called from multiple threads. GetMainFrame doesn't
trigger any notifications so it shouldn't need that lock. Instead, only
use NotificationStateLock on the UI thread.
This commit is contained in:
Marshall Greenblatt 2021-09-16 11:01:55 +03:00
parent db807e804a
commit 84117f2d1b
2 changed files with 8 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
libcef/browser/browser_info.cc
View File

@ -234,7 +234,7 @@ void CefBrowserInfo::RemoveFrame(content::RenderFrameHost* host) {
}
CefRefPtr<CefFrameHostImpl> CefBrowserInfo::GetMainFrame() {
NotificationStateLock lock_scope(this);
base::AutoLock lock_scope(lock_);
// Early exit if called post-destruction.
if (!browser_ || is_closing_)
return nullptr;
@ -504,6 +504,8 @@ void CefBrowserInfo::RemoveAllFrames(
CefBrowserInfo::NotificationStateLock::NotificationStateLock(
CefBrowserInfo* browser_info)
: browser_info_(browser_info) {
CEF_REQUIRE_UIT();
// Take the navigation state lock.
{
base::AutoLock lock_scope_(browser_info_->notification_lock_);
@ -518,6 +520,8 @@ CefBrowserInfo::NotificationStateLock::NotificationStateLock(
}
CefBrowserInfo::NotificationStateLock::~NotificationStateLock() {
CEF_REQUIRE_UIT();
// Unlock in reverse order.
browser_info_lock_scope_.reset();
@ -530,11 +534,8 @@ CefBrowserInfo::NotificationStateLock::~NotificationStateLock() {
if (!queue_.empty()) {
DCHECK(frame_handler_);
scoped_refptr<NavigationLock> nav_lock;
if (CEF_CURRENTLY_ON_UIT()) {
// Don't navigate while inside callbacks.
nav_lock = browser_info_->CreateNavigationLock();
}
// Don't navigate while inside callbacks.
auto nav_lock = browser_info_->CreateNavigationLock();
// Empty the queue of pending actions. Any of these actions might result in
// the acquisition of a new NotificationStateLock.

2
libcef/browser/browser_info.h
View File

@ -204,7 +204,7 @@ class CefBrowserInfo : public base::RefCountedThreadSafe<CefBrowserInfo> {
// Used instead of |base::AutoLock(lock_)| in situations that might generate
// CefFrameHandler notifications. Any notifications passed to
// MaybeExecuteFrameNotification() will be queued until the lock is released,
// and then executed in order.
// and then executed in order. Only accessed on the UI thread.
class NotificationStateLock final {
public:
explicit NotificationStateLock(CefBrowserInfo* browser_info);