diff options
Diffstat (limited to '0005-ozone-wayland-Use-mutex-before-accessing-surfaces-ma.patch')
| -rw-r--r-- | 0005-ozone-wayland-Use-mutex-before-accessing-surfaces-ma.patch | 276 |
1 files changed, 276 insertions, 0 deletions
diff --git a/0005-ozone-wayland-Use-mutex-before-accessing-surfaces-ma.patch b/0005-ozone-wayland-Use-mutex-before-accessing-surfaces-ma.patch new file mode 100644 index 000000000000..9a4e8e822cb3 --- /dev/null +++ b/0005-ozone-wayland-Use-mutex-before-accessing-surfaces-ma.patch @@ -0,0 +1,276 @@ +From fd1d8bfd55d5b1d073cbad3ebe8f058c49f3702a Mon Sep 17 00:00:00 2001 +From: Maksim Sisov <msisov@igalia.com> +Date: Mon, 5 Aug 2019 16:14:47 +0300 +Subject: [PATCH 5/5] [ozone/wayland] Use mutex before accessing surfaces map. + +We must make sure that accessing surfaces map from different threads +is safe. Otherwise, this is subject to races and unexpected crashes. + +Bug: 987950 +Change-Id: I2e70e9c1ad48943be518c3571b7ca1fb91f8d51b +--- + ui/ozone/platform/wayland/BUILD.gn | 1 + + .../wayland/gpu/gbm_pixmap_wayland.cc | 6 +- + .../wayland/gpu/gbm_surfaceless_wayland.cc | 2 +- + .../wayland/gpu/wayland_buffer_manager_gpu.cc | 61 ++++++++----------- + .../wayland/gpu/wayland_buffer_manager_gpu.h | 14 ++++- + .../wayland/gpu/wayland_canvas_surface.cc | 2 +- + .../wayland/gpu/wayland_surface_gpu.cc | 16 +++++ + .../wayland/gpu/wayland_surface_gpu.h | 13 +++- + 8 files changed, 72 insertions(+), 43 deletions(-) + create mode 100644 ui/ozone/platform/wayland/gpu/wayland_surface_gpu.cc + +diff --git a/ui/ozone/platform/wayland/BUILD.gn b/ui/ozone/platform/wayland/BUILD.gn +index 686138bb614a..a1560a20fe23 100644 +--- a/ui/ozone/platform/wayland/BUILD.gn ++++ b/ui/ozone/platform/wayland/BUILD.gn +@@ -30,6 +30,7 @@ source_set("wayland") { + "gpu/wayland_canvas_surface.h", + "gpu/wayland_surface_factory.cc", + "gpu/wayland_surface_factory.h", ++ "gpu/wayland_surface_gpu.cc", + "gpu/wayland_surface_gpu.h", + "host/wayland_buffer_manager_connector.cc", + "host/wayland_buffer_manager_connector.h", +diff --git a/ui/ozone/platform/wayland/gpu/gbm_pixmap_wayland.cc b/ui/ozone/platform/wayland/gpu/gbm_pixmap_wayland.cc +index 5d8167bdfd89..917f849811a2 100644 +--- a/ui/ozone/platform/wayland/gpu/gbm_pixmap_wayland.cc ++++ b/ui/ozone/platform/wayland/gpu/gbm_pixmap_wayland.cc +@@ -126,10 +126,10 @@ bool GbmPixmapWayland::ScheduleOverlayPlane( + const gfx::RectF& crop_rect, + bool enable_blend, + std::unique_ptr<gfx::GpuFence> gpu_fence) { +- auto* surface = buffer_manager_->GetSurface(widget); +- DCHECK(surface); ++ auto surface_weak = buffer_manager_->GetSurface(widget); ++ DCHECK(surface_weak); + GbmSurfacelessWayland* surfaceless = +- static_cast<GbmSurfacelessWayland*>(surface); ++ static_cast<GbmSurfacelessWayland*>(surface_weak.get()); + DCHECK(surfaceless); + + surfaceless->QueueOverlayPlane( +diff --git a/ui/ozone/platform/wayland/gpu/gbm_surfaceless_wayland.cc b/ui/ozone/platform/wayland/gpu/gbm_surfaceless_wayland.cc +index 70ea08acfa11..0554e772f5cb 100644 +--- a/ui/ozone/platform/wayland/gpu/gbm_surfaceless_wayland.cc ++++ b/ui/ozone/platform/wayland/gpu/gbm_surfaceless_wayland.cc +@@ -34,7 +34,7 @@ GbmSurfacelessWayland::GbmSurfacelessWayland( + has_implicit_external_sync_( + HasEGLExtension("EGL_ARM_implicit_external_sync")), + weak_factory_(this) { +- buffer_manager_->RegisterSurface(widget_, this); ++ buffer_manager_->RegisterSurface(widget_, GetWeakPtr()); + unsubmitted_frames_.push_back(std::make_unique<PendingFrame>()); + } + +diff --git a/ui/ozone/platform/wayland/gpu/wayland_buffer_manager_gpu.cc b/ui/ozone/platform/wayland/gpu/wayland_buffer_manager_gpu.cc +index 63bfa3032fde..f113a9887bdc 100644 +--- a/ui/ozone/platform/wayland/gpu/wayland_buffer_manager_gpu.cc ++++ b/ui/ozone/platform/wayland/gpu/wayland_buffer_manager_gpu.cc +@@ -43,19 +43,13 @@ void WaylandBufferManagerGpu::OnSubmission(gfx::AcceleratedWidget widget, + gfx::SwapResult swap_result) { + DCHECK(io_thread_runner_->BelongsToCurrentThread()); + DCHECK_NE(widget, gfx::kNullAcceleratedWidget); +- auto* surface = GetSurface(widget); +- // There can be a race between destruction and submitting the last frames. The +- // surface can be destroyed by the time the host receives a request to destroy +- // a buffer, and is able to call the OnSubmission for that specific buffer. +- if (surface) { +- // As long as mojo calls rerouted to the IO child thread, we have to reroute +- // them back to the same thread, where the original commit buffer call came +- // from. +- commit_thread_runner_->PostTask( +- FROM_HERE, +- base::Bind(&WaylandSurfaceGpu::OnSubmission, base::Unretained(surface), +- buffer_id, swap_result)); +- } ++ auto surface = GetSurface(widget); ++ // As long as mojo calls rerouted to the IO child thread, we have to reroute ++ // them back to the same thread, where the original commit buffer call came ++ // from. ++ commit_thread_runner_->PostTask( ++ FROM_HERE, base::Bind(&WaylandSurfaceGpu::OnSubmission, surface, ++ buffer_id, swap_result)); + } + + void WaylandBufferManagerGpu::OnPresentation( +@@ -64,36 +58,35 @@ void WaylandBufferManagerGpu::OnPresentation( + const gfx::PresentationFeedback& feedback) { + DCHECK(io_thread_runner_->BelongsToCurrentThread()); + DCHECK_NE(widget, gfx::kNullAcceleratedWidget); +- auto* surface = GetSurface(widget); +- // There can be a race between destruction and presenting the last frames. The +- // surface can be destroyed by the time the host receives a request to destroy +- // a buffer, and is able to call the OnPresentation for that specific buffer. +- if (surface) { +- // As long as mojo calls rerouted to the IO child thread, we have to reroute +- // them back to the same thread, where the original commit buffer call came +- // from. +- commit_thread_runner_->PostTask( +- FROM_HERE, base::Bind(&WaylandSurfaceGpu::OnPresentation, +- base::Unretained(surface), buffer_id, feedback)); +- } +-} +- +-void WaylandBufferManagerGpu::RegisterSurface(gfx::AcceleratedWidget widget, +- WaylandSurfaceGpu* surface) { ++ auto surface = GetSurface(widget); ++ // As long as mojo calls rerouted to the IO child thread, we have to reroute ++ // them back to the same thread, where the original commit buffer call came ++ // from. ++ commit_thread_runner_->PostTask( ++ FROM_HERE, base::Bind(&WaylandSurfaceGpu::OnPresentation, surface, ++ buffer_id, feedback)); ++} ++ ++void WaylandBufferManagerGpu::RegisterSurface( ++ gfx::AcceleratedWidget widget, ++ base::WeakPtr<WaylandSurfaceGpu> surface) { ++ base::AutoLock scoped_lock(lock_); + widget_to_surface_map_.insert(std::make_pair(widget, surface)); + } + + void WaylandBufferManagerGpu::UnregisterSurface(gfx::AcceleratedWidget widget) { ++ base::AutoLock scoped_lock(lock_); + widget_to_surface_map_.erase(widget); + } + +-WaylandSurfaceGpu* WaylandBufferManagerGpu::GetSurface( +- gfx::AcceleratedWidget widget) const { +- WaylandSurfaceGpu* surface = nullptr; ++base::WeakPtr<WaylandSurfaceGpu> WaylandBufferManagerGpu::GetSurface( ++ gfx::AcceleratedWidget widget) { ++ base::AutoLock scoped_lock(lock_); ++ + auto it = widget_to_surface_map_.find(widget); + if (it != widget_to_surface_map_.end()) +- surface = it->second; +- return surface; ++ return it->second; ++ return nullptr; + } + + void WaylandBufferManagerGpu::CreateDmabufBasedBuffer( +diff --git a/ui/ozone/platform/wayland/gpu/wayland_buffer_manager_gpu.h b/ui/ozone/platform/wayland/gpu/wayland_buffer_manager_gpu.h +index 87439610cfc3..631d715e719e 100644 +--- a/ui/ozone/platform/wayland/gpu/wayland_buffer_manager_gpu.h ++++ b/ui/ozone/platform/wayland/gpu/wayland_buffer_manager_gpu.h +@@ -8,6 +8,8 @@ + #include <memory> + + #include "base/macros.h" ++#include "base/memory/weak_ptr.h" ++#include "base/synchronization/lock.h" + #include "base/threading/sequenced_task_runner_handle.h" + #include "base/threading/thread_checker.h" + #include "mojo/public/cpp/bindings/associated_binding.h" +@@ -64,9 +66,11 @@ class WaylandBufferManagerGpu : public ozone::mojom::WaylandBufferManagerGpu { + // result of the below operations, they must register themselves with the + // below APIs. + void RegisterSurface(gfx::AcceleratedWidget widget, +- WaylandSurfaceGpu* surface); ++ base::WeakPtr<WaylandSurfaceGpu> surface); + void UnregisterSurface(gfx::AcceleratedWidget widget); +- WaylandSurfaceGpu* GetSurface(gfx::AcceleratedWidget widget) const; ++ // A client of this method must always get and acquire lock before doing ++ // any manipulations with the surface. Once done, the lock must be released. ++ base::WeakPtr<WaylandSurfaceGpu> GetSurface(gfx::AcceleratedWidget widget); + + // Methods, which can be used when in both in-process-gpu and out of process + // modes. These calls are forwarded to the browser process through the +@@ -156,7 +160,8 @@ class WaylandBufferManagerGpu : public ozone::mojom::WaylandBufferManagerGpu { + mojo::AssociatedBinding<ozone::mojom::WaylandBufferManagerGpu> + associated_binding_; + +- std::map<gfx::AcceleratedWidget, WaylandSurfaceGpu*> widget_to_surface_map_; ++ std::map<gfx::AcceleratedWidget, base::WeakPtr<WaylandSurfaceGpu>> ++ widget_to_surface_map_; // Guarded by |lock_|. + + // This task runner can be used to pass messages back to the same thread, + // where the commit buffer request came from. For example, swap requests come +@@ -171,6 +176,9 @@ class WaylandBufferManagerGpu : public ozone::mojom::WaylandBufferManagerGpu { + // needed to ensure mojo calls happen on a right sequence. + scoped_refptr<base::SingleThreadTaskRunner> io_thread_runner_; + ++ // Protects access to |widget_to_surface_map_|. ++ base::Lock lock_; ++ + DISALLOW_COPY_AND_ASSIGN(WaylandBufferManagerGpu); + }; + +diff --git a/ui/ozone/platform/wayland/gpu/wayland_canvas_surface.cc b/ui/ozone/platform/wayland/gpu/wayland_canvas_surface.cc +index 6de24d7fd177..ce44ebac2915 100644 +--- a/ui/ozone/platform/wayland/gpu/wayland_canvas_surface.cc ++++ b/ui/ozone/platform/wayland/gpu/wayland_canvas_surface.cc +@@ -29,7 +29,7 @@ WaylandCanvasSurface::WaylandCanvasSurface( + WaylandBufferManagerGpu* buffer_manager, + gfx::AcceleratedWidget widget) + : buffer_manager_(buffer_manager), widget_(widget) { +- buffer_manager_->RegisterSurface(widget_, this); ++ buffer_manager_->RegisterSurface(widget_, GetWeakPtr()); + } + + WaylandCanvasSurface::~WaylandCanvasSurface() { +diff --git a/ui/ozone/platform/wayland/gpu/wayland_surface_gpu.cc b/ui/ozone/platform/wayland/gpu/wayland_surface_gpu.cc +new file mode 100644 +index 000000000000..85296edcca42 +--- /dev/null ++++ b/ui/ozone/platform/wayland/gpu/wayland_surface_gpu.cc +@@ -0,0 +1,16 @@ ++// Copyright 2019 The Chromium Authors. All rights reserved. ++// Use of this source code is governed by a BSD-style license that can be ++// found in the LICENSE file. ++ ++#include "ui/ozone/platform/wayland/gpu/wayland_surface_gpu.h" ++ ++namespace ui { ++ ++WaylandSurfaceGpu::WaylandSurfaceGpu() = default; ++WaylandSurfaceGpu::~WaylandSurfaceGpu() = default; ++ ++base::WeakPtr<WaylandSurfaceGpu> WaylandSurfaceGpu::GetWeakPtr() { ++ return weak_factory_.GetWeakPtr(); ++} ++ ++} // namespace ui +diff --git a/ui/ozone/platform/wayland/gpu/wayland_surface_gpu.h b/ui/ozone/platform/wayland/gpu/wayland_surface_gpu.h +index ace5279e838e..38d285317ab3 100644 +--- a/ui/ozone/platform/wayland/gpu/wayland_surface_gpu.h ++++ b/ui/ozone/platform/wayland/gpu/wayland_surface_gpu.h +@@ -8,6 +8,7 @@ + #include <memory> + + #include "base/macros.h" ++#include "base/memory/weak_ptr.h" + + namespace gfx { + enum class SwapResult; +@@ -22,7 +23,7 @@ namespace ui { + // the buffer. + class WaylandSurfaceGpu { + public: +- virtual ~WaylandSurfaceGpu() {} ++ virtual ~WaylandSurfaceGpu(); + + // Tells the surface the result of the last swap of buffer with the + // |buffer_id|. +@@ -33,6 +34,16 @@ class WaylandSurfaceGpu { + // |buffer_id|. + virtual void OnPresentation(uint32_t buffer_id, + const gfx::PresentationFeedback& feedback) = 0; ++ ++ protected: ++ WaylandSurfaceGpu(); ++ ++ base::WeakPtr<WaylandSurfaceGpu> GetWeakPtr(); ++ ++ private: ++ base::WeakPtrFactory<WaylandSurfaceGpu> weak_factory_{this}; ++ ++ DISALLOW_COPY_AND_ASSIGN(WaylandSurfaceGpu); + }; + + } // namespace ui +-- +2.22.0 + |