Lock the initialization of the GL bindings.

Lock the initialization of the GL bindings.

The GpuInfoCollector and CompositorGL exist on different threads in the browser process and both use the GL bindings. I've witnessed a race in the binding initialization where one thread thinks that the bindings have been initialized, but the initialization hasn't completed on the other thread, and a NULL function pointer gets dereferenced.

BUG=none
TEST=fixes segfault on TOUCH_UI build

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=98280

[jonathan.backer, 2011-08-24 16:48:36]

The bug that this fixes reproduces consistently on my machine. I've made the
change TOUCH_UI specific to have minimal impact. A different solution is to have
the bindings initialized earlier. I'm happy to work in that direction, but I'd
like to land this as a stop-gap. I've made the change conditional on the
platform that is affected by the bug.

[jonathan.backer, 2011-08-24 22:01:31]

-apatrick@ who is out
+kbr@ in his stead

[Ken Russell (switch to Gerrit), 2011-08-25 04:19:53]

If you want I will approve this as is, but it would be better to come up with a
solution that didn't violate the style guide and add more global initialization
code.

http://codereview.chromium.org/7724003/diff/1/ui/gfx/gl/gl_implementation_lin...
File ui/gfx/gl/gl_implementation_linux.cc (right):

http://codereview.chromium.org/7724003/diff/1/ui/gfx/gl/gl_implementation_lin...
ui/gfx/gl/gl_implementation_linux.cc:50: base::Lock g_lock;
This isn't legal according to the Google C++ style guide -- see
http://google-styleguide.googlecode.com/svn/trunk/cppguide.xml#Static_and_Glo...
. Could you use an Atomic32 and base::subtle::NoBarrier_CompareAndSwap and a
spin loop instead? Or some other mechanism?

[jonathan.backer, 2011-08-25 12:59:02]

On 2011/08/25 04:19:53, kbr wrote:
> If you want I will approve this as is, but it would be better to come up with
a
> solution that didn't violate the style guide and add more global
initialization
> code.

I'd like to keep this as is. base::Lock is a very useful utility with platform
specific backends. I looked a bit and on Linux, base::Lock is using pthreads
mutexes. I'm certain this is more sophisticated than a spin lock. Moreover, a
'git gs' shows that this pattern is used elsewhere:
base/mime_util_xdg.cc:31
base/time_win.cc:294
chrome_frame/crash_reporting/crash_report.cc:17
chrome_frame/protocol_sink_wrap.h:140
chrome/browser/rlz/rlz.cc:47
...

So I'm not setting precedent. Just following it.

[Ken Russell (switch to Gerrit), 2011-08-25 18:32:43]

OK. LGTM

[jam, 2011-08-25 20:23:33]

http://codereview.chromium.org/7724003/diff/5001/ui/gfx/gl/gl_implementation_...
File ui/gfx/gl/gl_implementation_linux.cc (right):

http://codereview.chromium.org/7724003/diff/5001/ui/gfx/gl/gl_implementation_...
ui/gfx/gl/gl_implementation_linux.cc:50: base::Lock g_lock;
this goes against the chromium/google style guide:
http://www.chromium.org/developers/coding-style#Static_Variables

[jam, 2011-08-29 21:00:57]

ping