[TTVD] Fix crash when deinitializing surface after conflict

It's possible that resource conflict and process termination
are handled in parallel. If resource conflict was handled
before surface deinitialization, it tried to once again
follow standard removing path and access surface instance.
Since it was already released during resource conflict
handling, result is segmentation fault.

This commit hardens logic inside OutputSurfaceState to
properly handle such scenarios.

Bug: https://jira-eu.sec.samsung.net/browse/VDGAME-544
Change-Id: I80fa71e4776f0daf38edbc34aaf8c9fe9ae72bb7
Signed-off-by: Jakub Gajownik <j.gajownik2@samsung.com>

diff --git a/tizen_src/chromium_impl/ui/ozone/platform/efl/output_surface_state.cc b/tizen_src/chromium_impl/ui/ozone/platform/efl/output_surface_state.cc
index 78bb0019a48c5536b48eb2f0342ce6828afded08..47d2f56af4883cd60b3b9db79aea1088830869b5 100644 (file)
--- a/tizen_src/chromium_impl/ui/ozone/platform/efl/output_surface_state.cc
+++ b/tizen_src/chromium_impl/ui/ozone/platform/efl/output_surface_state.cc
@@ -15,6 +15,30 @@ constexpr base::TimeDelta kSurfaceInactivityThreshold = base::Milliseconds(500);
 
 namespace ui {
 
+namespace {
+std::ostream& operator<<(std::ostream& stream,
+                         OutputSurfaceState::State state) {
+  switch (state) {
+    case OutputSurfaceState::State::kNotInitialized:
+      stream << "not initialized";
+      break;
+    case OutputSurfaceState::State::kError:
+      stream << "error";
+      break;
+    case OutputSurfaceState::State::kActive:
+      stream << "active";
+      break;
+    case OutputSurfaceState::State::kIdle:
+      stream << "idle";
+      break;
+    case OutputSurfaceState::State::kRemoving:
+      stream << "removing";
+      break;
+  }
+  return stream;
+}
+}  // namespace
+
 bool OutputSurfaceState::Acquire(
     bool allow_allocation,
     base::OnceCallback<void(OutputSurface::Plane)> conflict_cb,
@@ -100,6 +124,20 @@ void OutputSurfaceState::ScheduleRemoving(
 void OutputSurfaceState::Deinitialize(base::OnceClosure cb) {
   TIZEN_MEDIA_LOG(INFO) << "Deinitialize surface: " << plane_;
 
+  switch (state_) {
+    case State::kError:
+      std::move(cb).Run();
+      return;
+    case State::kNotInitialized:
+    case State::kRemoving:
+      TIZEN_MEDIA_LOG(ERROR) << "Unexpected state: " << state_;
+      std::move(cb).Run();
+      return;
+    case State::kIdle:
+    case State::kActive:
+      break;
+  }
+
   state_ = State::kRemoving;
   removing_timer_.Stop();
   Unregister();
@@ -108,8 +146,21 @@ void OutputSurfaceState::Deinitialize(base::OnceClosure cb) {
 
 void OutputSurfaceState::Reset() {
   TIZEN_MEDIA_LOG(INFO) << "Remove surface: " << plane_;
-  surface_->ReleaseResource();
-  surface_ = nullptr;
+
+  switch (state_) {
+    case State::kError:
+      break;
+    case State::kNotInitialized:
+    case State::kIdle:
+    case State::kActive:
+      TIZEN_MEDIA_LOG(ERROR) << "Unexpected state: " << state_;
+      return;
+    case State::kRemoving:
+      surface_->ReleaseResource();
+      surface_ = nullptr;
+      break;
+  }
+
   state_ = State::kNotInitialized;
 }
 

diff --git a/tizen_src/chromium_impl/ui/ozone/platform/efl/output_surface_state.h b/tizen_src/chromium_impl/ui/ozone/platform/efl/output_surface_state.h
index b7278faeabda5c7ba9b89d684e8eff4f21b5fede..d9984c58fc06606a9e3b5b6b8c92c6d5a1d0d486 100644 (file)
--- a/tizen_src/chromium_impl/ui/ozone/platform/efl/output_surface_state.h
+++ b/tizen_src/chromium_impl/ui/ozone/platform/efl/output_surface_state.h
@@ -39,16 +39,24 @@ class OutputSurfaceState {
       base::OnceClosure cb,
       scoped_refptr<base::SequencedTaskRunner> task_runner = nullptr);
   void Deinitialize(base::OnceClosure cb);
+  // Completely removes surface, it won't be longer usable. Note that it should
+  // be done only after callback passed to |ScheduleRemoving| is triggered.
   void Reset();
   // Notify instance that resource underneath was taken and it should not
   // allow further accessing it.
   void MarkResourceTaken();
 
   enum class State {
+    // Surface is not initialized and resource is not acquired.
     kNotInitialized,
+    // After resource conflict or internal error of surface, cannot be longer
+    // reused.
     kError,
     kActive,
+    // Surface should be there, but it's inactive for some time. Note that
+    // |removing_timer_| should be active.
     kIdle,
+    // Surface is under remove, it should not be used without reacquiring.
     kRemoving,
   };