From a5af1a78c1b12f32464da13145c81ec5de74c01e Mon Sep 17 00:00:00 2001
From: Joshua Ashton <joshua@froggi.es>
Date: Sat, 11 Dec 2021 07:04:41 +0000
Subject: [PATCH] steamcompmgr: Avoid deadlock between import_commit and
 destroy_buffer

Very rarely we can see a deadlock between import_commit and destroy_buffer where:

- destroy_buffer is waiting on `wlr_buffer_map_lock` and has `wlserver_lock` (from up the chain)

- import_commit has `wlr_buffer_map_lock` and is waiting on `wlserver_lock`

To avoid this, we simply replace the lock_guard in import_commit with a unique_lock and manually unlock this before going into the section where we need to lock the wl_server.

This is safe for a few reasons:
- 1: All accesses to wlr_buffer_map are done before this lock.
- 2: destroy_buffer cannot be called from this buffer before now
     as it only happens because of the signal added below.
- 3: "References to elements in the unordered_map container remain
	  valid in all cases, even after a rehash."
---
 src/steamcompmgr.cpp | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/src/steamcompmgr.cpp b/src/steamcompmgr.cpp
index fa53064..1d42935 100644
--- a/src/steamcompmgr.cpp
+++ b/src/steamcompmgr.cpp
@@ -661,10 +661,7 @@ release_commit( commit_t &commit )
 static bool
 import_commit ( struct wlr_buffer *buf, commit_t &commit )
 {
-	/* Keep this locked during the entire function so we
-	 * avoid racing the creation of the commit. Unlikely to
-	 * come up with the current callers but a safe default. */
-	std::lock_guard<std::mutex> lock( wlr_buffer_map_lock );
+	std::unique_lock<std::mutex> lock( wlr_buffer_map_lock );
 
 	commit.buf = buf;
 
@@ -674,6 +671,12 @@ import_commit ( struct wlr_buffer *buf, commit_t &commit )
 		commit.vulkanTex = it->second.vulkanTex;
 		commit.fb_id = it->second.fb_id;
 
+		/* Unlock here to avoid deadlock [1],
+		 * drm_lock_fbid calls wlserver_lock.
+		 * Map is no longer used here and the element 
+		 * is no longer accessed. */
+		lock.unlock();
+
 		if (commit.fb_id)
 		{
 			drm_lock_fbid( &g_DRM, commit.fb_id );
@@ -682,6 +685,18 @@ import_commit ( struct wlr_buffer *buf, commit_t &commit )
 		return true;
 	}
 
+	wlr_buffer_map_entry& entry = wlr_buffer_map[buf];
+	/* [1]
+	 * Need to unlock the wlr_buffer_map_lock to avoid
+	 * a deadlock on destroy_buffer if it owns the wlserver_lock.
+	 * This is safe for a few reasons:
+	 * - 1: All accesses to wlr_buffer_map are done before this lock.
+	 * - 2: destroy_buffer cannot be called from this buffer before now
+	 *      as it only happens because of the signal added below.
+	 * - 3: "References to elements in the unordered_map container remain
+	 *		 valid in all cases, even after a rehash." */
+	lock.unlock();
+
 	commit.vulkanTex = vulkan_create_texture_from_wlr_buffer( buf );
 	assert( commit.vulkanTex != 0 );
 
@@ -700,7 +715,6 @@ import_commit ( struct wlr_buffer *buf, commit_t &commit )
 		commit.fb_id = 0;
 	}
 
-	wlr_buffer_map_entry& entry = wlr_buffer_map[buf];
 	entry.listener.notify = destroy_buffer;
 	entry.buf = buf;
 	entry.vulkanTex = commit.vulkanTex;