Title: [291480] trunk/Source/WebCore
- Revision
- 291480
- Author
- [email protected]
- Date
- 2022-03-18 06:30:31 -0700 (Fri, 18 Mar 2022)
Log Message
[Linux] GBMDevice as a thread-specific object is problematic
https://bugs.webkit.org/show_bug.cgi?id=237758
<rdar://problem/90476889>
Patch by Zan Dobersek <[email protected]> on 2022-03-18
Reviewed by Alejandro G. Castro.
Per-thread GBMDevice instances (and the internal gbm_device objects) can
end up causing trouble if the gbm_bo objects spawned from it are moved
across threads and used after the originating thread (and that thread's
GBMDevice instance) is destroyed. This can happen with a GStreamer
pipeline where repeated playback is possibly moved across different
threads.
The original intention was to preemptively avoid any thread-safety
issues in the libgbm implementations but we haven't seen any yet. That
might change down the line once we adapt against implementations from
different vendors, at which point the most sensible solution would be
to deploy a specific thread on which all libgbm operations would be
handled.
* platform/graphics/gbm/GBMBufferSwapchain.cpp:
(WebCore::GBMBufferSwapchain::getBuffer):
* platform/graphics/gbm/GBMDevice.cpp:
(WebCore::GBMDevice::singleton):
(WebCore::GBMDevice::GBMDevice):
(WebCore::GBMDevice::~GBMDevice):
(WebCore::threadSpecificDevice): Deleted.
(WebCore::GBMDevice::get): Deleted.
* platform/graphics/gbm/GBMDevice.h:
* platform/graphics/texmap/GraphicsContextGLTextureMapperANGLE.cpp:
(WebCore::GraphicsContextGLTextureMapper::platformInitialize):
(WebCore::GraphicsContextGLANGLE::EGLImageBacking::reset):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (291479 => 291480)
--- trunk/Source/WebCore/ChangeLog 2022-03-18 12:59:28 UTC (rev 291479)
+++ trunk/Source/WebCore/ChangeLog 2022-03-18 13:30:31 UTC (rev 291480)
@@ -1,3 +1,38 @@
+2022-03-18 Zan Dobersek <[email protected]>
+
+ [Linux] GBMDevice as a thread-specific object is problematic
+ https://bugs.webkit.org/show_bug.cgi?id=237758
+ <rdar://problem/90476889>
+
+ Reviewed by Alejandro G. Castro.
+
+ Per-thread GBMDevice instances (and the internal gbm_device objects) can
+ end up causing trouble if the gbm_bo objects spawned from it are moved
+ across threads and used after the originating thread (and that thread's
+ GBMDevice instance) is destroyed. This can happen with a GStreamer
+ pipeline where repeated playback is possibly moved across different
+ threads.
+
+ The original intention was to preemptively avoid any thread-safety
+ issues in the libgbm implementations but we haven't seen any yet. That
+ might change down the line once we adapt against implementations from
+ different vendors, at which point the most sensible solution would be
+ to deploy a specific thread on which all libgbm operations would be
+ handled.
+
+ * platform/graphics/gbm/GBMBufferSwapchain.cpp:
+ (WebCore::GBMBufferSwapchain::getBuffer):
+ * platform/graphics/gbm/GBMDevice.cpp:
+ (WebCore::GBMDevice::singleton):
+ (WebCore::GBMDevice::GBMDevice):
+ (WebCore::GBMDevice::~GBMDevice):
+ (WebCore::threadSpecificDevice): Deleted.
+ (WebCore::GBMDevice::get): Deleted.
+ * platform/graphics/gbm/GBMDevice.h:
+ * platform/graphics/texmap/GraphicsContextGLTextureMapperANGLE.cpp:
+ (WebCore::GraphicsContextGLTextureMapper::platformInitialize):
+ (WebCore::GraphicsContextGLANGLE::EGLImageBacking::reset):
+
2022-03-18 Kimmo Kinnunen <[email protected]>
Recycling a webgl context when it has been lost and restored causes a crash
Modified: trunk/Source/WebCore/platform/graphics/gbm/GBMBufferSwapchain.cpp (291479 => 291480)
--- trunk/Source/WebCore/platform/graphics/gbm/GBMBufferSwapchain.cpp 2022-03-18 12:59:28 UTC (rev 291479)
+++ trunk/Source/WebCore/platform/graphics/gbm/GBMBufferSwapchain.cpp 2022-03-18 13:30:31 UTC (rev 291480)
@@ -44,7 +44,7 @@
RefPtr<GBMBufferSwapchain::Buffer> GBMBufferSwapchain::getBuffer(const BufferDescription& description)
{
- auto& device = GBMDevice::get();
+ auto& device = GBMDevice::singleton();
// If the description of the requested buffers has changed, update the description to the new one and wreck the existing buffers.
// This should handle changes in format or dimension of the buffers.
Modified: trunk/Source/WebCore/platform/graphics/gbm/GBMDevice.cpp (291479 => 291480)
--- trunk/Source/WebCore/platform/graphics/gbm/GBMDevice.cpp 2022-03-18 12:59:28 UTC (rev 291479)
+++ trunk/Source/WebCore/platform/graphics/gbm/GBMDevice.cpp 2022-03-18 13:30:31 UTC (rev 291480)
@@ -32,32 +32,25 @@
#include <fcntl.h>
#include <gbm.h>
#include <mutex>
-#include <wtf/ThreadSpecific.h>
+#include <wtf/StdLibExtras.h>
#include <xf86drm.h>
namespace WebCore {
-static ThreadSpecific<GBMDevice>& threadSpecificDevice()
+const GBMDevice& GBMDevice::singleton()
{
- static ThreadSpecific<GBMDevice>* s_gbmDevice;
+ static std::unique_ptr<GBMDevice> s_device;
static std::once_flag s_onceFlag;
std::call_once(s_onceFlag,
[] {
- s_gbmDevice = new ThreadSpecific<GBMDevice>;
+ s_device = makeUnique<GBMDevice>();
});
- return *s_gbmDevice;
+ return *s_device;
}
-const GBMDevice& GBMDevice::get()
-{
- return *threadSpecificDevice();
-}
-
GBMDevice::GBMDevice()
{
- static int s_globalFd { -1 };
- static std::once_flag s_onceFlag;
- std::call_once(s_onceFlag, [] {
+ [&] {
drmDevicePtr devices[64];
memset(devices, 0, sizeof(devices));
@@ -70,24 +63,24 @@
if (!(device->available_nodes & (1 << DRM_NODE_RENDER)))
continue;
- s_globalFd = open(device->nodes[DRM_NODE_RENDER], O_RDWR | O_CLOEXEC);
- if (s_globalFd >= 0)
+ m_fd = open(device->nodes[DRM_NODE_RENDER], O_RDWR | O_CLOEXEC);
+ if (m_fd >= 0)
break;
}
drmFreeDevices(devices, numDevices);
- });
+ }();
- if (s_globalFd >= 0)
- m_device = gbm_create_device(s_globalFd);
+ if (m_fd >= 0)
+ m_device = gbm_create_device(m_fd);
}
GBMDevice::~GBMDevice()
{
- if (m_device) {
+ if (m_device)
gbm_device_destroy(m_device);
- m_device = nullptr;
- }
+ if (m_fd >= 0)
+ close(m_fd);
}
} // namespace WebCore
Modified: trunk/Source/WebCore/platform/graphics/gbm/GBMDevice.h (291479 => 291480)
--- trunk/Source/WebCore/platform/graphics/gbm/GBMDevice.h 2022-03-18 12:59:28 UTC (rev 291479)
+++ trunk/Source/WebCore/platform/graphics/gbm/GBMDevice.h 2022-03-18 13:30:31 UTC (rev 291480)
@@ -38,7 +38,7 @@
WTF_MAKE_NONCOPYABLE(GBMDevice);
WTF_MAKE_FAST_ALLOCATED();
public:
- static const GBMDevice& get();
+ static const GBMDevice& singleton();
GBMDevice();
~GBMDevice();
@@ -46,6 +46,7 @@
struct gbm_device* device() const { return m_device; }
private:
+ int m_fd { -1 };
struct gbm_device* m_device { nullptr };
};
Modified: trunk/Source/WebCore/platform/graphics/texmap/GraphicsContextGLTextureMapperANGLE.cpp (291479 => 291480)
--- trunk/Source/WebCore/platform/graphics/texmap/GraphicsContextGLTextureMapperANGLE.cpp 2022-03-18 12:59:28 UTC (rev 291479)
+++ trunk/Source/WebCore/platform/graphics/texmap/GraphicsContextGLTextureMapperANGLE.cpp 2022-03-18 13:30:31 UTC (rev 291480)
@@ -163,7 +163,7 @@
m_nicosiaLayer = makeUnique<Nicosia::GCGLANGLELayer>(*this);
m_layerContentsDisplayDelegate = PlatformLayerDisplayDelegate::create(&m_nicosiaLayer->contentLayer());
- const auto& gbmDevice = GBMDevice::get();
+ const auto& gbmDevice = GBMDevice::singleton();
if (gbmDevice.device()) {
m_textureBacking = makeUnique<EGLImageBacking>(platformDisplay());
m_compositorTextureBacking = makeUnique<EGLImageBacking>(platformDisplay());
@@ -290,7 +290,7 @@
if (!width || !height)
return false;
- const auto& gbmDevice = GBMDevice::get();
+ const auto& gbmDevice = GBMDevice::singleton();
m_BO = gbm_bo_create(gbmDevice.device(), width, height, hasAlpha ? GBM_BO_FORMAT_ARGB8888 : GBM_BO_FORMAT_XRGB8888, GBM_BO_USE_RENDERING);
if (m_BO) {
m_FD = gbm_bo_get_fd(m_BO);
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
