dnkl/foot
41
2.0k
Fork
You've already forked foot
243

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

Manually merged
dnkl merged 1 commit from shm-buffer-busy-sometimes-not-set into master 2021年12月18日 14:41:27 +01:00
Owner
Copy link

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 the newly selected buffer. 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 release it. And there, in our buffer_release() callback, we’d assert that buffer->busy was set. And fail.

@dankamongmen I haven't yet been able to reproduce the crash, but I'm fairly sure this is it. Any chance you can give this branch a spin?

Closes #844

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 the newly selected buffer. 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 release it. And there, in our `buffer_release()` callback, we’d assert that `buffer->busy` was set. And fail. @dankamongmen I haven't yet been able to reproduce the crash, but I'm fairly sure this is it. Any chance you can give this branch a spin? Closes #844
dnkl force-pushed shm-buffer-busy-sometimes-not-set from ae3426c735
All checks were successful
continuous-integration/woodpecker the build was successful
to b88ccb8ea8
All checks were successful
continuous-integration/woodpecker the build was successful
2021年12月16日 13:03:44 +01:00
Compare
dnkl force-pushed shm-buffer-busy-sometimes-not-set from b88ccb8ea8
All checks were successful
continuous-integration/woodpecker the build was successful
to fc6533c920
All checks were successful
continuous-integration/woodpecker the build was successful
2021年12月18日 14:26:29 +01:00
Compare
dnkl manually merged commit 0b9cb6bd2f into master 2021年12月18日 14:41:27 +01:00
Contributor
Copy link

indeed, i can no longer reproduce this crash. good work!

indeed, i can no longer reproduce this crash. good work!
Sign in to join this conversation.
No reviewers
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
dnkl/foot!845
Reference in a new issue
dnkl/foot
No description provided.
Delete branch "shm-buffer-busy-sometimes-not-set"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?