Skip to content

xserver: prevent repeated Present wait-fence callbacks - #1114

Open
KiralyCraft wants to merge 1 commit into
termux:masterfrom
KiralyCraft:fix/present-wait-fence-requeue
Open

xserver: prevent repeated Present wait-fence callbacks#1114
KiralyCraft wants to merge 1 commit into
termux:masterfrom
KiralyCraft:fix/present-wait-fence-requeue

Conversation

@KiralyCraft

Copy link
Copy Markdown

This fixes a Termux:X11 crash by disarming a Present wait fence callback before reexecution so the same Present request cannot be invoked repeatedly. Required by lfdevs/mesa-for-android-container#96.

@KiralyCraft

Copy link
Copy Markdown
Author

It looks like this upstream commit may be related: https://gitlab.freedesktop.org/xorg/xserver/-/commit/f19ab94ba9c891d801231654267556dc7f32b5e0

@robertkirkman

Copy link
Copy Markdown
Member

Is the bug this fixes reproducible without using proot-distro or chroot?

@KiralyCraft

Copy link
Copy Markdown
Author

In theory yes, since it is triggered by X11 Present and Sync requests handled inside Termux:X11. I currently only reproduced it using the #96 Mesa build from a chroot. A native Termux XCB client should be able to reproduce it by submitting PresentPixmap with an unsignaled wait fence and then triggering that fence while the request is queued for a future MSC; However, I did not yet run these tests yet. We are though following the specs.

The Present protocol explicitly allows a client to pass an unsignaled X Sync fence as PresentPixmap’s wait-fence. It requires the X server to defer presentation until that fence is triggered. Scheduling the same presentation for a future MSC is also valid, so the server must wait for both conditions. Present protocol specification (see here) says "“PresentPixmap will block until 'wait-fence' is triggered."

The native Mesa build shipped by Termux does use DRI3 and Present, but it does not normally use this particular mechanism. It supplies no wait-fence and instead uses an idle-fence, which X triggers after it has finished accessing the pixmap so Mesa knows the buffer can be reused. The PR i referenced adds the opposite synchronization direction by converting the KGSL rendering completion fence into an X Sync wait-fence. Normal applications using the native Termux Mesa package are unlikely to use this exact callback path, although it remains a valid upstream bug imo.

miSyncTriggerFence() restarts its trigger-list scan after every fired
callback because the callback may modify that list. Disarm this
callback before re-executing: re_execute() may queue the request for
a future MSC and leave the Present fence alive, which would otherwise
make this same trigger fire repeatedly in the restarted scan.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants