pacificIT / chromiumembedded

Automatically exported from code.google.com/p/chromiumembedded
0 stars 1 forks source link

CEF1: Add OnRegisterCustomSchemes callback to address url_util thread safety issues #630

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
I haven't been able to come up with a simple reproducer for this because it 
appears to be a multi-threading race condition.  I've seen a number of crashes 
that have the following in the stack trace:

6   libcef  url_util::`anonymous namespace'::InitStandardSchemes()  0x5c 
    src\googleurl\src\url_util.cc @ 88
7   libcef  url_util::`anonymous namespace'::DoIsStandard<char>...  0x17 
    src\googleurl\src\url_util.cc @ 112
8   libcef  `anonymous namespace'::CefUrlRequestManager::AddFactory(...     0xf3 
    src\cef\libcef\scheme_impl.cc @ 384
9   libcef  CefRegisterSchemeHandlerFactory(class CefStringBase<CefSt...    0x86 
    src\cef\libcef\scheme_impl.cc @ 610

What version of the product are you using? On what operating system?

This was observed with CEF revisions 283 and 441.

Please provide any additional information below.

Registering the scheme handler factory eventually calls into DoIsStandard() 
which calls InitStandardSchemes() (googleurl\src\url_util.cc).  If 
url_util::Initialize() has not been called yet, InitStandardSchemes() will try 
to do lazy initialization.  This only works in single threaded environments.  
Here is the comment from Initialize() in url_util.h:

// Initialization is NOT required, it will be implicitly initialized when first
// used. However, this implicit initialization is NOT threadsafe. If you are
// using this library in a threaded environment and don't have a consistent
// "first call" (an example might be calling "AddStandardScheme" with your
// special application-specific schemes) then you will want to call initialize
// before spawning any threads.

As far as I can tell, CEF does not call Initialize() and because it runs in a 
multi-threaded environment I'm wondering if this is the source of the crashes 
I'm seeing.

Could adding a call to url_util::initialize() in CefInitialize() or some other 
part of the initialization sequence help with this?

Original issue reported on code.google.com by avivr...@gmail.com on 6 Jun 2012 at 1:00

GoogleCodeExporter commented 9 years ago
The underlying problem is that standard scheme initialization 
(AddStandardScheme, InitStandardSchemes) is not thread safe. All standard 
scheme registration should take place before the first browser window is 
created. In CEF3 this is enforced by use of CefApp::OnRegisterCustomSchemes. We 
should do something similar for CEF1 as well.

In the mean time you can work around the problem by only calling 
CefRegisterCustomScheme (or CefRegisterSchemeHandlerFactory if 
CefRegisterCustomScheme isn't used) when no browser windows exist (ie, at 
application startup). You can then call CefRegisterSchemeHandlerFactory again 
at a later time and it will be safe.

Original comment by magreenb...@gmail.com on 7 Jun 2012 at 8:00

GoogleCodeExporter commented 9 years ago
"In the mean time you can work around the problem by only calling 
CefRegisterCustomScheme (or CefRegisterSchemeHandlerFactory if 
CefRegisterCustomScheme isn't used) when no browser windows exist (ie, at 
application startup)"

This is how my application was set up when I was seeing the crashes.  During 
initialization I do the following:

CefInitialize()
CefRegisterPlugin() // register a few plugins
CefRegisterCustomScheme() // I repeat this line and the next line to register a 
couple of custom schemes
CefRegisterSchemeHandlerFactory()

Later on (after initialization) I call CefBrowser::CreateBrowser() for the 
first time.

Original comment by avivr...@gmail.com on 8 Jun 2012 at 1:41

GoogleCodeExporter commented 9 years ago
What operating system are you using? If Windows, are you using the 
multi_threaded_message_loop setting?

Original comment by magreenb...@gmail.com on 8 Jun 2012 at 3:49

GoogleCodeExporter commented 9 years ago
I'm using Windows 7, and I am using the multi_threaded_message_loop setting

Original comment by avivr...@gmail.com on 8 Jun 2012 at 6:09

GoogleCodeExporter commented 9 years ago
> I'm using Windows 7, and I am using the multi_threaded_message_loop setting

OK, in the case of multi_threaded_message_loop the UI and IO message loops are 
both already running when CefRegisterCustomScheme is called, which explains how 
this crash can occur. With a multi_threaded_message_loop setting of false the 
UI message loop doesn't start until CefRunMessageLoop or CefDoMessageLoopWork 
is called.

Original comment by magreenb...@gmail.com on 8 Jun 2012 at 6:12

GoogleCodeExporter commented 9 years ago
Thanks for the explanation.  Do you have any suggestions for a workaround if I 
still want to use multi_threaded_message_loop?

Original comment by avivr...@gmail.com on 8 Jun 2012 at 6:15

GoogleCodeExporter commented 9 years ago
Unfortunately there's no solution for that situation short of fixing the bug.

Original comment by magreenb...@gmail.com on 8 Jun 2012 at 6:25

GoogleCodeExporter commented 9 years ago

Original comment by magreenb...@gmail.com on 3 Oct 2012 at 6:04

GoogleCodeExporter commented 9 years ago
[deleted comment]
GoogleCodeExporter commented 9 years ago
Issue 788 has been merged into this issue.

Original comment by magreenb...@gmail.com on 27 Nov 2012 at 4:04

GoogleCodeExporter commented 9 years ago
(Sorry for spamming the patches on this issue.) This is what we're going with 
for now in our build, in order to fix this problem until we upgrade to a CEF 
release with the callback: a mutex in googleurl/src/url_util.cc guarding access 
to standard_schemes. It may be overkill or unfeasible to add this in release 
branches, but here it is for consideration anyway.

Original comment by camden.l...@gmail.com on 28 Nov 2012 at 12:32

Attachments:

GoogleCodeExporter commented 9 years ago
New CefApp::OnRegisterCustomSchemes callback added in trunk revision 1115 and 
1364 branch revision 1116.

Original comment by magreenb...@gmail.com on 4 Mar 2013 at 8:19