- Revision
- 205734
- Author
- [email protected]
- Date
- 2016-09-09 03:53:50 -0700 (Fri, 09 Sep 2016)
Log Message
[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:
Modified Paths
Diff
Modified: releases/WebKitGTK/webkit-2.14/Source/WebCore/ChangeLog (205733 => 205734)
--- 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)
@@ -1,3 +1,32 @@
+2016-09-06 Carlos Garcia Campos <[email protected]>
+
+ [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:
+
2016-09-06 Saam Barati <[email protected]>
Make JSMap and JSSet faster
Modified: releases/WebKitGTK/webkit-2.14/Source/WebCore/platform/graphics/GLContext.cpp (205733 => 205734)
--- 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)
@@ -54,54 +54,6 @@
return *ThreadGlobalGLContext::staticGLContext;
}
-#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)
-
static bool initializeOpenGLShimsIfNeeded()
{
#if USE(OPENGL_ES_2)
@@ -178,10 +130,6 @@
GLContext::GLContext(PlatformDisplay& display)
: m_display(display)
{
-#if PLATFORM(X11)
- if (display.type() == PlatformDisplay::Type::X11)
- addActiveContext(this);
-#endif
}
GLContext::~GLContext()
@@ -188,10 +136,6 @@
{
if (this == currentContext()->context())
currentContext()->setContext(nullptr);
-#if PLATFORM(X11)
- if (m_display.type() == PlatformDisplay::Type::X11)
- removeActiveContext(this);
-#endif
}
bool GLContext::makeContextCurrent()
Modified: releases/WebKitGTK/webkit-2.14/Source/WebCore/platform/graphics/glx/GLContextGLX.cpp (205733 => 205734)
--- 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)
@@ -25,6 +25,9 @@
#include "PlatformDisplayX11.h"
#include <GL/glx.h>
#include <cairo.h>
+#include <cstdlib>
+#include <wtf/HashSet.h>
+#include <wtf/NeverDestroyed.h>
#if ENABLE(ACCELERATED_2D_CANVAS)
#include <cairo-gl.h>
@@ -32,6 +35,25 @@
namespace WebCore {
+// 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;
+}
+
#if !defined(PFNGLXSWAPINTERVALSGIPROC)
typedef int (*PFNGLXSWAPINTERVALSGIPROC) (int);
#endif
@@ -161,6 +183,7 @@
, m_context(WTFMove(context))
, m_window(window)
{
+ activeContexts().add(this);
}
GLContextGLX::GLContextGLX(PlatformDisplay& display, XUniqueGLXContext&& context, XUniqueGLXPbuffer&& pbuffer)
@@ -168,6 +191,7 @@
, m_context(WTFMove(context))
, m_pbuffer(WTFMove(pbuffer))
{
+ activeContexts().add(this);
}
GLContextGLX::GLContextGLX(PlatformDisplay& display, XUniqueGLXContext&& context, XUniquePixmap&& pixmap, XUniqueGLXPixmap&& glxPixmap)
@@ -176,19 +200,31 @@
, m_pixmap(WTFMove(pixmap))
, m_glxPixmap(WTFMove(glxPixmap))
{
+ activeContexts().add(this);
}
GLContextGLX::~GLContextGLX()
{
- if (m_cairoDevice)
+ clear();
+ activeContexts().remove(this);
+}
+
+void GLContextGLX::clear()
+{
+ if (!m_context)
+ return;
+
+ if (m_cairoDevice) {
cairo_device_destroy(m_cairoDevice);
+ m_cairoDevice = nullptr;
+ }
- 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);
- }
+ // 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;
}
bool GLContextGLX::canRenderToDefaultFramebuffer()
Modified: releases/WebKitGTK/webkit-2.14/Source/WebCore/platform/graphics/glx/GLContextGLX.h (205733 => 205734)
--- 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)
@@ -53,6 +53,8 @@
PlatformGraphicsContext3D platformContext() override;
#endif
+ void clear();
+
private:
GLContextGLX(PlatformDisplay&, XUniqueGLXContext&&, XID);
GLContextGLX(PlatformDisplay&, XUniqueGLXContext&&, XUniqueGLXPbuffer&&);
