Fix SocketHandleWaiter condition_variable bug

Condition variables can un-block themselves occasionally. This patch
fixes a possible segfault that can happen in the above case.

Change-Id: Ib00a4f25eb7ebc65c7e361b703bb5af527257e4a
Reviewed-on: https://chromium-review.googlesource.com/c/openscreen/+/1825872
Commit-Queue: Ryan Keane <rwkeane@google.com>
Reviewed-by: Jordan Bayles <jophba@chromium.org>

2 files changed, 19 insertions, 2 deletions

diff --git a/platform/impl/socket_handle_waiter.cc b/platform/impl/socket_handle_waiter.cc
index 11a7b57d..0022165f 100644
--- a/platform/impl/socket_handle_waiter.cc
+++ b/platform/impl/socket_handle_waiter.cc
@@ -49,11 +49,17 @@ void SocketHandleWaiter::OnHandleDeletion(Subscriber* subscriber,
   if (it != handle_mappings_.end()) {
     handle_mappings_.erase(it);
     if (!disable_locking_for_testing) {
+      handles_being_deleted_.push_back(handle);
+
       // This code will allow us to block completion of the socket destructor
       // (and subsequent invalidation of pointers to this socket) until we no
       // longer are waiting on a SELECT(...) call to it, since we only signal
       // this condition variable's wait(...) to proceed outside of SELECT(...).
-      handle_deletion_block_.wait(lock);
+      handle_deletion_block_.wait(lock, [this, handle]() {
+        return std::find(handles_being_deleted_.begin(),
+                         handles_being_deleted_.end(),
+                         handle) == handles_being_deleted_.end();
+      });
     }
   }
 }
@@ -76,6 +82,7 @@ Error SocketHandleWaiter::ProcessHandles(const Clock::duration& timeout) {
   std::vector<SocketHandleRef> handles;
   {
     std::lock_guard<std::mutex> lock(mutex_);
+    handles_being_deleted_.clear();
     handle_deletion_block_.notify_all();
     handles.reserve(handle_mappings_.size());
     for (const auto& pair : handle_mappings_) {
@@ -85,7 +92,13 @@ Error SocketHandleWaiter::ProcessHandles(const Clock::duration& timeout) {
 
   ErrorOr<std::vector<SocketHandleRef>> changed_handles =
       AwaitSocketsReadable(handles, timeout);
-  handle_deletion_block_.notify_all();
+
+  {
+    std::lock_guard<std::mutex> lock(mutex_);
+    handles_being_deleted_.clear();
+    handle_deletion_block_.notify_all();
+  }
+
   if (changed_handles.is_error()) {
     return changed_handles.error();
   }
diff --git a/platform/impl/socket_handle_waiter.h b/platform/impl/socket_handle_waiter.h
index 69eccc4f..1501d047 100644
--- a/platform/impl/socket_handle_waiter.h
+++ b/platform/impl/socket_handle_waiter.h
@@ -85,6 +85,10 @@ class SocketHandleWaiter {
   // Blocks deletion of handles until they are no longer being watched.
   std::condition_variable handle_deletion_block_;
 
+  // Set of handles currently being deleted, for ensuring handle_deletion_block_
+  // does not exit prematurely.
+  std::vector<SocketHandleRef> handles_being_deleted_;
+
   // Set of all socket handles currently being watched, mapped to the subscriber
   // that is watching them.
   std::unordered_map<SocketHandleRef, Subscriber*, SocketHandleHash>