Skip to content

Commit

Permalink
fix: forcing reinit of stuck IPC mutex, fixing cursor stuttering
Browse files Browse the repository at this point in the history
For some reason cursor updates do not trigged renderer updates if there were no pending root window texture updates.
Explicit signaling renderer thread in the case of cursor movement will fix it.

Preparing to move renderer from X server process to activity.
There are no robust mutexes support in bionic so in the case if X server or activity dies (activity is finished, force-stopped, segfaulted, interrupted, etc) with locked mutex other side will never be able to unlock it.
In this case we should explicitly reinitialize/replace mutex with new, fresh and unlocked mutex and try to lock it again.
  • Loading branch information
twaik committed Jan 7, 2025
1 parent f5e0f05 commit 7e130e4
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 14 deletions.
20 changes: 10 additions & 10 deletions app/src/main/cpp/lorie/InitOutput.c
Original file line number Diff line number Diff line change
Expand Up @@ -313,11 +313,12 @@ static Bool resetRootCursor(unused ClientPtr pClient, unused void *closure) {
}

static void lorieMoveCursor(unused DeviceIntPtr pDev, unused ScreenPtr pScr, int x, int y) {
// No need to lock for such an easy operation
// just change it and a signal will be sent on the next choreographer tick.
pvfb->state->cursor.x = x;
pvfb->state->cursor.y = y;
pvfb->state->cursor.moved = TRUE;
// No need to explicitly lock the mutex, it will cause waiting for rendering to be finished.
// We are simply signaling the renderer in the case if it sleeps.
pthread_cond_signal(&pvfb->state->cond);
}

static void lorieConvertCursor(CursorPtr pCurs, uint32_t *data) {
Expand Down Expand Up @@ -355,7 +356,7 @@ static void lorieSetCursor(unused DeviceIntPtr pDev, unused ScreenPtr pScr, Curs
return;
}

pthread_mutex_lock(&pvfb->state->cursor.lock);
lorie_mutex_lock(&pvfb->state->cursor.lock);
if (pCurs && bits) {
pvfb->state->cursor.xhot = bits->xhot;
pvfb->state->cursor.yhot = bits->yhot;
Expand All @@ -367,10 +368,9 @@ static void lorieSetCursor(unused DeviceIntPtr pDev, unused ScreenPtr pScr, Curs
pvfb->state->cursor.width = pvfb->state->cursor.height = 0;
}
pvfb->state->cursor.updated = true;
pthread_mutex_unlock(&pvfb->state->cursor.lock);
lorie_mutex_unlock(&pvfb->state->cursor.lock);

if (x0 >= 0 && y0 >= 0)
lorieMoveCursor(NULL, NULL, x0, y0);
lorieMoveCursor(NULL, NULL, x0, y0);
}

static miPointerSpriteFuncRec loriePointerSpriteFuncs = {
Expand All @@ -389,7 +389,7 @@ static miPointerScreenFuncRec loriePointerCursorFuncs = {
};

static void lorieUpdateBuffer(void) {
pthread_mutex_lock(&pvfb->state->lock);
lorie_mutex_lock(&pvfb->state->lock);
AHardwareBuffer_Desc desc;
LorieBuffer *new = NULL, *old = pvfb->root.buffer;
int status = 0;
Expand Down Expand Up @@ -422,7 +422,7 @@ static void lorieUpdateBuffer(void) {
}
LorieBuffer_release(old);
}
pthread_mutex_unlock(&pvfb->state->lock);
lorie_mutex_unlock(&pvfb->state->lock);

renderer_set_buffer(pvfb->env, new);
}
Expand Down Expand Up @@ -791,8 +791,8 @@ static const GCFuncs lorieGCFuncs = {
lorieValidateGC, lorieChangeGC, lorieCopyGC, lorieDestroyGC, lorieChangeClip, lorieDestroyClip, lorieCopyClip
};

#define LOCK_DRAWABLE(a) if (a == pScreenPtr->devPrivate || (a && a->type == DRAWABLE_WINDOW)) pthread_mutex_lock(&pvfb->state->lock)
#define UNLOCK_DRAWABLE(a) if (a == pScreenPtr->devPrivate || (a && a->type == DRAWABLE_WINDOW)) pthread_mutex_unlock(&pvfb->state->lock)
#define LOCK_DRAWABLE(a) if (a == pScreenPtr->devPrivate || (a && a->type == DRAWABLE_WINDOW)) lorie_mutex_lock(&pvfb->state->lock)
#define UNLOCK_DRAWABLE(a) if (a == pScreenPtr->devPrivate || (a && a->type == DRAWABLE_WINDOW)) lorie_mutex_unlock(&pvfb->state->lock)

#define GC_OP_VOID_DEF(name, argdefs, args) \
static void lorie ## name argdefs { \
Expand Down
34 changes: 34 additions & 0 deletions app/src/main/cpp/lorie/lorie.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,40 @@ __unused void renderer_set_buffer(JNIEnv* env, LorieBuffer* buffer);
__unused void renderer_set_window(JNIEnv* env, jobject surface);
__unused void renderer_set_shared_state(struct lorie_shared_server_state* state);

static inline __always_inline void lorie_mutex_lock(pthread_mutex_t* mutex) {
// Unfortunately there is no robust mutexes in bionic.
// Posix does not define any valid way to unlock stuck non-robust mutex
// so in the case if renderer or X server process unexpectedly die with locked mutex
// we will simply reinitialize it.
struct timespec ts = {0};
while(true) {
clock_gettime(CLOCK_MONOTONIC, &ts);

// 33 msec is enough to complete any drawing operation on both X server and renderer side
// In the case if mutex is locked most likely other thread died with the mutex locked
ts.tv_nsec += 33UL * 1000000UL;
if (ts.tv_nsec >= 1000000000L) {
ts.tv_sec += ts.tv_nsec / 1000000000L;
ts.tv_nsec = ts.tv_nsec % 1000000000L;
}

if (pthread_mutex_timedlock(mutex, &ts) == ETIMEDOUT && !lorieConnectionAlive()) {
pthread_mutexattr_t attr;
pthread_mutex_t initializer = PTHREAD_MUTEX_INITIALIZER;
pthread_mutexattr_init(&attr);
pthread_mutexattr_setpshared(&attr, PTHREAD_PROCESS_SHARED);
pthread_mutexattr_settype(&attr, PTHREAD_MUTEX_RECURSIVE);
memcpy(mutex, &initializer, sizeof(initializer));
pthread_mutex_init(mutex, &attr);
// Mutex will be locked fine on the next iteration
} else return;
}
}

static inline __always_inline void lorie_mutex_unlock(pthread_mutex_t* mutex) {
pthread_mutex_unlock(mutex);
}

typedef enum {
EVENT_UNKNOWN,
EVENT_SHARED_SERVER_STATE,
Expand Down
10 changes: 6 additions & 4 deletions app/src/main/cpp/lorie/renderer.c
Original file line number Diff line number Diff line change
Expand Up @@ -547,8 +547,10 @@ static void renderer_renew_image(void) {
int renderer_redraw_locked(JNIEnv* env) {
int err = EGL_SUCCESS;

glFinish();

// We should signal X server to not use root window or change cursor while we actively use them
pthread_mutex_lock(&state->lock);
lorie_mutex_lock(&state->lock);
// Non-null display.desc.data means we have root window created in legacy drawing mode so we should update it on each frame.
if (display.desc.data && state->drawRequested) {
state->drawRequested = FALSE;
Expand All @@ -561,15 +563,15 @@ int renderer_redraw_locked(JNIEnv* env) {
draw(display.id, -1.f, -1.f, 1.f, 1.f, display.desc.format != AHARDWAREBUFFER_FORMAT_B8G8R8A8_UNORM);

glFinish();
pthread_mutex_unlock(&state->lock);
lorie_mutex_unlock(&state->lock);

if (state->cursor.updated) {
log("Xlorie: updating cursor\n");
pthread_mutex_lock(&state->cursor.lock);
lorie_mutex_lock(&state->cursor.lock);
state->cursor.updated = false;
bindLinearTexture(cursor.id);
glTexImage2D(GL_TEXTURE_2D, 0, GL_RGBA, (GLsizei) state->cursor.width, (GLsizei) state->cursor.height, 0, GL_RGBA, GL_UNSIGNED_BYTE, state->cursor.bits);
pthread_mutex_unlock(&state->cursor.lock);
lorie_mutex_unlock(&state->cursor.lock);
}

state->cursor.moved = FALSE;
Expand Down

0 comments on commit 7e130e4

Please sign in to comment.