[wasm][caching] Fix CacheEventWaiter ownership 13/291813/8
authorJakub Gajownik <j.gajownik2@samsung.com>
Tue, 18 Apr 2023 10:12:46 +0000 (12:12 +0200)
committerBot Blink <blinkbot@samsung.com>
Fri, 21 Apr 2023 14:50:38 +0000 (14:50 +0000)
[Problem]
Crash during WebAssembly module deserialization from cache.

[Cause]
Access to raw pointer to CacheEventWaiter which was already
deleted.

[Solution]
Ensure lifetime of |cache_event_waiter_| is hold by
|CacheReaderTask|.
Additionally there is possibility that wire bytes fetching
will never finish (aborted by streaming compilation), so
proper handling has been added for such scenario.

Bug: https://cam.sprc.samsung.pl/browse/VDWASM-1224
Change-Id: Ic2894ec001ff2b6502300b83dc0e22b361bc4990

tizen_src/chromium_impl/v8/src/wasm/wasm-module-object-cache.cc
tizen_src/chromium_impl/v8/src/wasm/wasm-module-object-cache.h
v8/src/wasm/module-compiler.cc

index baf5dda2025e764ec9ba76ccf0eb4edce2a0b975..a677a485f22274335fa3d372b21efd1cf44689f8 100644 (file)
@@ -645,6 +645,53 @@ bool CacheWriter::SetAllDataWritten() {
   return true;
 }
 
+struct EventWaiter::EventState {
+  base::Mutex mutex;
+  base::ConditionVariable condition_var;
+  enum class Result { kNone, kSignaled, kCanceled };
+  Result result;
+
+  void SetResult(Result new_result) {
+    {
+      base::MutexGuard guard(&mutex);
+      if (result != EventWaiter::EventState::Result::kNone) {
+        return;
+      }
+      result = new_result;
+    }
+    condition_var.NotifyAll();
+  }
+};
+
+bool EventWaiter::WaitForEvent() {
+  base::MutexGuard guard(&state_->mutex);
+  while (state_->result == EventState::Result::kNone) {
+    state_->condition_var.Wait(&state_->mutex);
+  }
+  return state_->result == EventState::Result::kSignaled;
+}
+
+EventWaiter::EventWaiter(std::shared_ptr<EventState> state) : state_(state) {}
+
+EventProvider::EventProvider()
+    : state_(std::make_shared<EventWaiter::EventState>()) {}
+
+EventProvider::~EventProvider() {
+  Cancel();
+}
+
+EventWaiter EventProvider::GetWaiter() {
+  return EventWaiter(state_);
+}
+
+void EventProvider::SignalEvent() {
+  state_->SetResult(EventWaiter::EventState::Result::kSignaled);
+}
+
+void EventProvider::Cancel() {
+  state_->SetResult(EventWaiter::EventState::Result::kCanceled);
+}
+
 }  // namespace WasmModuleObjectCache
 }  // namespace internal
 }  // namespace v8
index dd0eeaef1ee740087bb328e473ce9cce0b703653..a72614c379358f5cc4ae8e20982904f81fcb376b 100644 (file)
@@ -370,16 +370,12 @@ class CacheWriter {
   bool all_requested_data_written_{false};
 };
 
-/**
- * @brief Provides means for waiting for declared events.
- * If the event was delivered before WaitForEvent() is called, it returns right
- * away. Otherwise it will wait until the event is delivered.
- * @tparam EventType Type of the event.
- */
-template <typename EventType>
+// Provides means for waiting for specific event. If the
+// event was delivered or canceled before `WaitForEvent`
+// is called, it returns right away. Otherwise it will wait
+// until the event is delivered or canceled.
 class EventWaiter {
  public:
-  EventWaiter() = default;
   ~EventWaiter() = default;
 
   EventWaiter(EventWaiter&&) = default;
@@ -388,41 +384,47 @@ class EventWaiter {
   EventWaiter(const EventWaiter&) = delete;
   EventWaiter& operator=(const EventWaiter&) = delete;
 
-  /**
-   * @brief Wait for event.
-   * It returns right away if the event was delivered prior to this call.
-   * Otherwise it waits for the expected event.
-   * @param event Expected event.
-   */
-  void WaitForEvent(const EventType& event);
-
-  /**
-   * @brief Call this method do signal that the expected event is delivered.
-   * @param event The event to be delivered.
-   */
-  void SignalEvent(const EventType& event);
+  // Wait for event signalation or cancelation. It returns
+  // right away if the event was already delivered or canceled
+  // prior to this call. Otherwise it wait for the action.
+  // Returns `true` if event was signaled or `false` in case
+  // event was canceled and there is no sense waiting for it
+  // (will block forever).
+  bool WaitForEvent();
 
  private:
-  using EventSet = std::unordered_set<EventType>;
-  EventSet finished_events_;
-  base::Mutex mutex_;
-  base::ConditionVariable condition_var_;
+  class EventState;
+
+  explicit EventWaiter(std::shared_ptr<EventState> state);
+
+  std::shared_ptr<EventState> state_;
+
+  friend class EventProvider;
 };
 
-template <typename EventType>
-void EventWaiter<EventType>::WaitForEvent(const EventType& event) {
-  base::MutexGuard guard(&mutex_);
-  while (finished_events_.find(event) == finished_events_.end()) {
-    condition_var_.Wait(&mutex_);
-  }
-}
-
-template <typename EventType>
-void EventWaiter<EventType>::SignalEvent(const EventType& event) {
-  base::MutexGuard guard(&mutex_);
-  finished_events_.insert(event);
-  condition_var_.NotifyAll();
-}
+class EventProvider {
+ public:
+  EventProvider();
+  ~EventProvider();
+
+  EventProvider(EventProvider&&) = default;
+  EventProvider& operator=(EventProvider&&) = default;
+
+  EventProvider(const EventProvider&) = delete;
+  EventProvider& operator=(const EventProvider&) = delete;
+
+  EventWaiter GetWaiter();
+
+  // Call this method to signal that the expected event is delivered.
+  void SignalEvent();
+
+  // Call this method interrupts waiters with indication that event
+  // will not be signaled at any time.
+  void Cancel();
+
+ private:
+  std::shared_ptr<EventWaiter::EventState> state_;
+};
 
 }  // namespace WasmModuleObjectCache
 }  // namespace internal
index e76da4b59c968f9af68f48c5e04e047666e5eb00..31e3842e26e523477971bf61fe5e04ef2672de9c 100644 (file)
@@ -1913,16 +1913,6 @@ void AsyncCompileJob::Abort() {
   GetWasmEngine()->RemoveCompileJob(this);
 }
 
-#if defined(TIZEN_TV_WASM_CACHING)
-namespace {
-enum class CacheEvent : uint8_t {
-  kWireBytesReady,
-};
-
-using CacheEventWaiter = WasmModuleObjectCache::EventWaiter<CacheEvent>;
-}  // namespace
-#endif  // TIZEN_TV_WASM_CACHING
-
 class AsyncStreamingProcessor final : public StreamingProcessor {
  public:
   explicit AsyncStreamingProcessor(AsyncCompileJob* job,
@@ -1982,7 +1972,7 @@ class AsyncStreamingProcessor final : public StreamingProcessor {
   // duplicate modules.
   size_t prefix_hash_;
 #if defined(TIZEN_TV_WASM_CACHING)
-  CacheEventWaiter cache_event_waiter_;
+  WasmModuleObjectCache::EventProvider cache_event_provider_;
 #endif
 };
 
@@ -2885,10 +2875,10 @@ class CacheReaderTask : public Task {
  public:
   CacheReaderTask(const std::string& cache_file_path,
                   std::weak_ptr<AsyncCompileJob> compile_job,
-                  CacheEventWaiter* cache_event_waiter)
+                  WasmModuleObjectCache::EventWaiter cache_event_waiter)
       : cache_file_path_(cache_file_path),
         compile_job_(compile_job),
-        cache_event_waiter_(cache_event_waiter) {}
+        cache_event_waiter_(std::move(cache_event_waiter)) {}
 
   void Run() override {
     if (auto compile_job = compile_job_.lock()) {
@@ -2909,8 +2899,10 @@ class CacheReaderTask : public Task {
       DLOG(DLOG_DEBUG) << "Export wrappers compilation finished";
       DLOG(DLOG_DEBUG) <<
           "Waiting for full wire bytes before instantiation";
-      CHECK_NE(nullptr, cache_event_waiter_);
-      cache_event_waiter_->WaitForEvent(CacheEvent::kWireBytesReady);
+      if (!cache_event_waiter_.WaitForEvent()) {
+        DLOG(DLOG_DEBUG) << "Wire bytes fetching aborted";
+        return;
+      }
       DLOG(DLOG_DEBUG) << "Full wire bytes received";
       if (result == DeserializationResult::SOME_FUNCTIONS_MISSING) {
         DLOG(DLOG_DEBUG) << "Waiting for baseline compilation finish, because "
@@ -2932,7 +2924,7 @@ class CacheReaderTask : public Task {
  private:
   const std::string cache_file_path_;
   std::weak_ptr<AsyncCompileJob> compile_job_;
-  CacheEventWaiter* const cache_event_waiter_;
+  WasmModuleObjectCache::EventWaiter cache_event_waiter_;
 };
 
 void AsyncStreamingProcessor::OnCodeSectionRead(
@@ -2941,8 +2933,10 @@ void AsyncStreamingProcessor::OnCodeSectionRead(
   if (FLAG_wasm_samsung_tv_caching && job_ && job_->native_module_) {
     const auto cache_file_path = WasmModuleObjectCache::GenerateModuleFilePath(
         wire_bytes);
-    auto task = std::make_unique<CacheReaderTask>(cache_file_path,
-        GetWasmEngine()->GetCompileJob(job_), &cache_event_waiter_);
+    auto task = std::make_unique<CacheReaderTask>(
+        cache_file_path,
+        GetWasmEngine()->GetCompileJob(job_),
+        cache_event_provider_.GetWaiter());
     // Try to read the module from cache in the background.
     V8::GetCurrentPlatform()->CallOnWorkerThread(std::move(task));
   }
@@ -3015,7 +3009,7 @@ void AsyncStreamingProcessor::OnFinishedStream(
   // Notify the CacheReaderTask that full wire bytes are now available
   // so it can move on to the module instantiation stage.
   if (FLAG_wasm_samsung_tv_caching)
-    cache_event_waiter_.SignalEvent(CacheEvent::kWireBytesReady);
+    cache_event_provider_.SignalEvent();
 #endif
   const bool needs_finish = job_->DecrementAndCheckFinisherCount();
   DCHECK_IMPLIES(!has_code_section, needs_finish);