<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[205734] releases/WebKitGTK/webkit-2.14/Source/WebCore</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/205734">205734</a></dd>
<dt>Author</dt> <dd>carlosgc@webkit.org</dd>
<dt>Date</dt> <dd>2016-09-09 03:53:50 -0700 (Fri, 09 Sep 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>[GTK] Crash of WebProcess on the last WebView disconnect
https://bugs.webkit.org/show_bug.cgi?id=161605
Reviewed by Michael Catanzaro.
The crash happens because GLX contexts are cleaned up in an exit handler to prevent X server crashes caused by
buggy drivers when process finishes with active GLX contexts. The cleanup is assuming that all contexts not
released when the exit handler is called are leaked, and then it manually deletes them. This assumption is no
longer true because PlatformDisplay owns the sharing GLContext now, and it's freed after the exit
handlers. Instead of deleting the GLContext objects, we could clear the internal GLXContext without breaking the
pointer ownership. Since this is specific to GLX, I've moed the code from GLContext to GLContextGLX and
simplified it.
* platform/graphics/GLContext.cpp:
(WebCore::GLContext::GLContext):
(WebCore::GLContext::~GLContext):
(WebCore::activeContextList): Deleted.
(WebCore::GLContext::addActiveContext): Deleted.
(WebCore::GLContext::removeActiveContext): Deleted.
(WebCore::GLContext::cleanupActiveContextsAtExit): Deleted.
* platform/graphics/glx/GLContextGLX.cpp:
(WebCore::activeContexts):
(WebCore::GLContextGLX::GLContextGLX):
(WebCore::GLContextGLX::~GLContextGLX):
(WebCore::GLContextGLX::clear):
* platform/graphics/glx/GLContextGLX.h:</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#releasesWebKitGTKwebkit214SourceWebCoreChangeLog">releases/WebKitGTK/webkit-2.14/Source/WebCore/ChangeLog</a></li>
<li><a href="#releasesWebKitGTKwebkit214SourceWebCoreplatformgraphicsGLContextcpp">releases/WebKitGTK/webkit-2.14/Source/WebCore/platform/graphics/GLContext.cpp</a></li>
<li><a href="#releasesWebKitGTKwebkit214SourceWebCoreplatformgraphicsglxGLContextGLXcpp">releases/WebKitGTK/webkit-2.14/Source/WebCore/platform/graphics/glx/GLContextGLX.cpp</a></li>
<li><a href="#releasesWebKitGTKwebkit214SourceWebCoreplatformgraphicsglxGLContextGLXh">releases/WebKitGTK/webkit-2.14/Source/WebCore/platform/graphics/glx/GLContextGLX.h</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="releasesWebKitGTKwebkit214SourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: releases/WebKitGTK/webkit-2.14/Source/WebCore/ChangeLog (205733 => 205734)</h4>
<pre class="diff"><span>
<span class="info">--- releases/WebKitGTK/webkit-2.14/Source/WebCore/ChangeLog 2016-09-09 10:48:21 UTC (rev 205733)
+++ releases/WebKitGTK/webkit-2.14/Source/WebCore/ChangeLog 2016-09-09 10:53:50 UTC (rev 205734)
</span><span class="lines">@@ -1,3 +1,32 @@
</span><ins>+2016-09-06 Carlos Garcia Campos <cgarcia@igalia.com>
+
+ [GTK] Crash of WebProcess on the last WebView disconnect
+ https://bugs.webkit.org/show_bug.cgi?id=161605
+
+ Reviewed by Michael Catanzaro.
+
+ The crash happens because GLX contexts are cleaned up in an exit handler to prevent X server crashes caused by
+ buggy drivers when process finishes with active GLX contexts. The cleanup is assuming that all contexts not
+ released when the exit handler is called are leaked, and then it manually deletes them. This assumption is no
+ longer true because PlatformDisplay owns the sharing GLContext now, and it's freed after the exit
+ handlers. Instead of deleting the GLContext objects, we could clear the internal GLXContext without breaking the
+ pointer ownership. Since this is specific to GLX, I've moed the code from GLContext to GLContextGLX and
+ simplified it.
+
+ * platform/graphics/GLContext.cpp:
+ (WebCore::GLContext::GLContext):
+ (WebCore::GLContext::~GLContext):
+ (WebCore::activeContextList): Deleted.
+ (WebCore::GLContext::addActiveContext): Deleted.
+ (WebCore::GLContext::removeActiveContext): Deleted.
+ (WebCore::GLContext::cleanupActiveContextsAtExit): Deleted.
+ * platform/graphics/glx/GLContextGLX.cpp:
+ (WebCore::activeContexts):
+ (WebCore::GLContextGLX::GLContextGLX):
+ (WebCore::GLContextGLX::~GLContextGLX):
+ (WebCore::GLContextGLX::clear):
+ * platform/graphics/glx/GLContextGLX.h:
+
</ins><span class="cx"> 2016-09-06 Saam Barati <sbarati@apple.com>
</span><span class="cx">
</span><span class="cx"> Make JSMap and JSSet faster
</span></span></pre></div>
<a id="releasesWebKitGTKwebkit214SourceWebCoreplatformgraphicsGLContextcpp"></a>
<div class="modfile"><h4>Modified: releases/WebKitGTK/webkit-2.14/Source/WebCore/platform/graphics/GLContext.cpp (205733 => 205734)</h4>
<pre class="diff"><span>
<span class="info">--- releases/WebKitGTK/webkit-2.14/Source/WebCore/platform/graphics/GLContext.cpp 2016-09-09 10:48:21 UTC (rev 205733)
+++ releases/WebKitGTK/webkit-2.14/Source/WebCore/platform/graphics/GLContext.cpp 2016-09-09 10:53:50 UTC (rev 205734)
</span><span class="lines">@@ -54,54 +54,6 @@
</span><span class="cx"> return *ThreadGlobalGLContext::staticGLContext;
</span><span class="cx"> }
</span><span class="cx">
</span><del>-#if PLATFORM(X11)
-// Because of driver bugs, exiting the program when there are active pbuffers
-// can crash the X server (this has been observed with the official Nvidia drivers).
-// We need to ensure that we clean everything up on exit. There are several reasons
-// that GraphicsContext3Ds will still be alive at exit, including user error (memory
-// leaks) and the page cache. In any case, we don't want the X server to crash.
-typedef Vector<GLContext*> ActiveContextList;
-static ActiveContextList& activeContextList()
-{
- DEPRECATED_DEFINE_STATIC_LOCAL(ActiveContextList, activeContexts, ());
- return activeContexts;
-}
-
-void GLContext::addActiveContext(GLContext* context)
-{
- static bool addedAtExitHandler = false;
- if (!addedAtExitHandler) {
- atexit(&GLContext::cleanupActiveContextsAtExit);
- addedAtExitHandler = true;
- }
- activeContextList().append(context);
-}
-
-static bool gCleaningUpAtExit = false;
-
-void GLContext::removeActiveContext(GLContext* context)
-{
- // If we are cleaning up the context list at exit, don't bother removing the context
- // from the list, since we don't want to modify the list while it's being iterated.
- if (gCleaningUpAtExit)
- return;
-
- ActiveContextList& contextList = activeContextList();
- size_t i = contextList.find(context);
- if (i != notFound)
- contextList.remove(i);
-}
-
-void GLContext::cleanupActiveContextsAtExit()
-{
- gCleaningUpAtExit = true;
-
- ActiveContextList& contextList = activeContextList();
- for (size_t i = 0; i < contextList.size(); ++i)
- delete contextList[i];
-}
-#endif // PLATFORM(X11)
-
</del><span class="cx"> static bool initializeOpenGLShimsIfNeeded()
</span><span class="cx"> {
</span><span class="cx"> #if USE(OPENGL_ES_2)
</span><span class="lines">@@ -178,10 +130,6 @@
</span><span class="cx"> GLContext::GLContext(PlatformDisplay& display)
</span><span class="cx"> : m_display(display)
</span><span class="cx"> {
</span><del>-#if PLATFORM(X11)
- if (display.type() == PlatformDisplay::Type::X11)
- addActiveContext(this);
-#endif
</del><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> GLContext::~GLContext()
</span><span class="lines">@@ -188,10 +136,6 @@
</span><span class="cx"> {
</span><span class="cx"> if (this == currentContext()->context())
</span><span class="cx"> currentContext()->setContext(nullptr);
</span><del>-#if PLATFORM(X11)
- if (m_display.type() == PlatformDisplay::Type::X11)
- removeActiveContext(this);
-#endif
</del><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> bool GLContext::makeContextCurrent()
</span></span></pre></div>
<a id="releasesWebKitGTKwebkit214SourceWebCoreplatformgraphicsglxGLContextGLXcpp"></a>
<div class="modfile"><h4>Modified: releases/WebKitGTK/webkit-2.14/Source/WebCore/platform/graphics/glx/GLContextGLX.cpp (205733 => 205734)</h4>
<pre class="diff"><span>
<span class="info">--- releases/WebKitGTK/webkit-2.14/Source/WebCore/platform/graphics/glx/GLContextGLX.cpp 2016-09-09 10:48:21 UTC (rev 205733)
+++ releases/WebKitGTK/webkit-2.14/Source/WebCore/platform/graphics/glx/GLContextGLX.cpp 2016-09-09 10:53:50 UTC (rev 205734)
</span><span class="lines">@@ -25,6 +25,9 @@
</span><span class="cx"> #include "PlatformDisplayX11.h"
</span><span class="cx"> #include <GL/glx.h>
</span><span class="cx"> #include <cairo.h>
</span><ins>+#include <cstdlib>
+#include <wtf/HashSet.h>
+#include <wtf/NeverDestroyed.h>
</ins><span class="cx">
</span><span class="cx"> #if ENABLE(ACCELERATED_2D_CANVAS)
</span><span class="cx"> #include <cairo-gl.h>
</span><span class="lines">@@ -32,6 +35,25 @@
</span><span class="cx">
</span><span class="cx"> namespace WebCore {
</span><span class="cx">
</span><ins>+// Because of driver bugs, exiting the program when there are active pbuffers
+// can crash the X server (this has been observed with the official Nvidia drivers).
+// We need to ensure that we clean everything up on exit. There are several reasons
+// that GraphicsContext3Ds will still be alive at exit, including user error (memory
+// leaks) and the page cache. In any case, we don't want the X server to crash.
+static HashSet<GLContextGLX*>& activeContexts()
+{
+ static std::once_flag onceFlag;
+ static LazyNeverDestroyed<HashSet<GLContextGLX*>> contexts;
+ std::call_once(onceFlag, [] {
+ contexts.construct();
+ std::atexit([] {
+ for (auto* context : activeContexts())
+ context->clear();
+ });
+ });
+ return contexts;
+}
+
</ins><span class="cx"> #if !defined(PFNGLXSWAPINTERVALSGIPROC)
</span><span class="cx"> typedef int (*PFNGLXSWAPINTERVALSGIPROC) (int);
</span><span class="cx"> #endif
</span><span class="lines">@@ -161,6 +183,7 @@
</span><span class="cx"> , m_context(WTFMove(context))
</span><span class="cx"> , m_window(window)
</span><span class="cx"> {
</span><ins>+ activeContexts().add(this);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> GLContextGLX::GLContextGLX(PlatformDisplay& display, XUniqueGLXContext&& context, XUniqueGLXPbuffer&& pbuffer)
</span><span class="lines">@@ -168,6 +191,7 @@
</span><span class="cx"> , m_context(WTFMove(context))
</span><span class="cx"> , m_pbuffer(WTFMove(pbuffer))
</span><span class="cx"> {
</span><ins>+ activeContexts().add(this);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> GLContextGLX::GLContextGLX(PlatformDisplay& display, XUniqueGLXContext&& context, XUniquePixmap&& pixmap, XUniqueGLXPixmap&& glxPixmap)
</span><span class="lines">@@ -176,19 +200,31 @@
</span><span class="cx"> , m_pixmap(WTFMove(pixmap))
</span><span class="cx"> , m_glxPixmap(WTFMove(glxPixmap))
</span><span class="cx"> {
</span><ins>+ activeContexts().add(this);
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> GLContextGLX::~GLContextGLX()
</span><span class="cx"> {
</span><del>- if (m_cairoDevice)
</del><ins>+ clear();
+ activeContexts().remove(this);
+}
+
+void GLContextGLX::clear()
+{
+ if (!m_context)
+ return;
+
+ if (m_cairoDevice) {
</ins><span class="cx"> cairo_device_destroy(m_cairoDevice);
</span><ins>+ m_cairoDevice = nullptr;
+ }
</ins><span class="cx">
</span><del>- if (m_context) {
- // This may be necessary to prevent crashes with NVidia's closed source drivers. Originally
- // from Mozilla's 3D canvas implementation at: http://bitbucket.org/ilmari/canvas3d/
- glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, 0);
- glXMakeCurrent(downcast<PlatformDisplayX11>(m_display).native(), None, None);
- }
</del><ins>+ // This may be necessary to prevent crashes with NVidia's closed source drivers. Originally
+ // from Mozilla's 3D canvas implementation at: http://bitbucket.org/ilmari/canvas3d/
+ glBindFramebufferEXT(GL_FRAMEBUFFER_EXT, 0);
+ glXMakeCurrent(downcast<PlatformDisplayX11>(m_display).native(), None, None);
+
+ m_context = nullptr;
</ins><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> bool GLContextGLX::canRenderToDefaultFramebuffer()
</span></span></pre></div>
<a id="releasesWebKitGTKwebkit214SourceWebCoreplatformgraphicsglxGLContextGLXh"></a>
<div class="modfile"><h4>Modified: releases/WebKitGTK/webkit-2.14/Source/WebCore/platform/graphics/glx/GLContextGLX.h (205733 => 205734)</h4>
<pre class="diff"><span>
<span class="info">--- releases/WebKitGTK/webkit-2.14/Source/WebCore/platform/graphics/glx/GLContextGLX.h 2016-09-09 10:48:21 UTC (rev 205733)
+++ releases/WebKitGTK/webkit-2.14/Source/WebCore/platform/graphics/glx/GLContextGLX.h 2016-09-09 10:53:50 UTC (rev 205734)
</span><span class="lines">@@ -53,6 +53,8 @@
</span><span class="cx"> PlatformGraphicsContext3D platformContext() override;
</span><span class="cx"> #endif
</span><span class="cx">
</span><ins>+ void clear();
+
</ins><span class="cx"> private:
</span><span class="cx"> GLContextGLX(PlatformDisplay&, XUniqueGLXContext&&, XID);
</span><span class="cx"> GLContextGLX(PlatformDisplay&, XUniqueGLXContext&&, XUniqueGLXPbuffer&&);
</span></span></pre>
</div>
</div>
</body>
</html>