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."
This commit is contained in:
Joshua Ashton 2021-12-11 07:04:41 +00:00 committed by Pierre-Loup A. Griffais
parent 952949f1a1
commit a5af1a78c1

View file

@ -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;