mirrors/foot
1
0
Fork 0
mirror of https://codeberg.org/dnkl/foot.git synced 2026-02-05 04:06:08 -05:00
Code Issues Projects Releases Packages Wiki Activity Actions

shm: get_buffer(): make sure buffer->busy is set

Browse source 
When going through the cached buffers, we only set buffer->busy on
the *first* re-usable buffer we found.

In some cases, we will find more than one re-usable buffer. In this
case, we select the “youngest” one (i.e the one most recently used, in
the hopes that we can use damage tracking instead of re-rendering the
entire buffer).

If the “current” buffer is younger than the previously detected,
re-usable, buffer, then we unref:ed the previously selected buffer,
and replaced it with the current one.

But, we did not sanitize it. That is, we did not:

* set buffer->busy
* clear its dirty region
* clear its scroll damage

That buffer would eventually get rendered to, and committed to the
compositor. Later, the compositor would free it. And there, in our
buffer_release() callback, we’d assert that buffer->busy was
set. And fail.

Closes #844
This commit is contained in:
Daniel Eklöf 2021-12-15 12:37:21 +01:00
parent fb398f473e
commit fc6533c920
No known key found for this signature in database
GPG key ID: 5BBD4992C116573F
2 changed files with 21 additions and 10 deletions
Download patch file Download diff file Expand all files Collapse all files

2
CHANGELOG.md
View file

@ -60,6 +60,8 @@
(https://codeberg.org/dnkl/foot/issues/842).
* Key presses triggering keyboard layout switches also being emitted
CSI codes in the Kitty keyboard protocol.
* Assertion in `shm.c:buffer_release()`
(https://codeberg.org/dnkl/foot/issues/844).
### Security

29
shm.c
View file

@ -546,15 +546,9 @@ shm_get_buffer(struct buffer_chain *chain, int width, int height)
else
#endif
{
if (cached == NULL) {
LOG_DBG("re-using buffer %p from cache", (void *)buf);
buf->busy = true;
pixman_region32_clear(&buf->public.dirty);
free(buf->public.scroll_damage);
buf->public.scroll_damage = NULL;
xassert(buf->public.pix_instances == chain->pix_instances);
cached = it->item;
} else {
if (cached == NULL)
cached = buf;
else {
/* We have multiple buffers eligible for
* re-use. Pick the “youngest” one, and mark the
* other one for purging */
@ -562,6 +556,14 @@ shm_get_buffer(struct buffer_chain *chain, int width, int height)
shm_unref(&cached->public);
cached = buf;
} else {
/*
* TODO: I think we _can_ use shm_unref()
* here...
*
* shm_unref() may remove ‘it’, but that
* should be safe; “our” tll_foreach() already
* holds the next pointer.
*/
if (buffer_unref_no_remove_from_chain(buf))
tll_remove(chain->bufs, it);
}
@ -569,8 +571,15 @@ shm_get_buffer(struct buffer_chain *chain, int width, int height)
}
}
if (cached != NULL)
if (cached != NULL) {
LOG_DBG("re-using buffer %p from cache", (void *)cached);
cached->busy = true;
pixman_region32_clear(&cached->public.dirty);
free(cached->public.scroll_damage);
cached->public.scroll_damage = NULL;
xassert(cached->public.pix_instances == chain->pix_instances);
return &cached->public;
}
struct buffer *ret;
get_new_buffers(chain, 1, &width, &height, &ret, false);