Skip to content

Commit

Permalink
[display] Add wait state to compositor switchover. JB#46746
Browse files Browse the repository at this point in the history
While ownership of well known compositor D-Bus name acts logically
as mutual exclusion lock that allows only one compositor instance,
timing wise name ownership change does not mean that underlying
resources used by previous compositor instance would already be
available for the current one - which can lead to resource allocation
failures and crashes for example when switching from one minui
based ui (unlock ui) to another one (upgrade ui).

Allow previous compositor instance up to five seconds to exit before
giving the current one permission to draw.

Signed-off-by: Simo Piiroinen <simo.piiroinen@jollamobile.com>
  • Loading branch information
spiiroin committed Sep 12, 2019
1 parent 1beea73 commit d42285d
Showing 1 changed file with 159 additions and 1 deletion.
160 changes: 159 additions & 1 deletion modules/display.c
Expand Up @@ -681,6 +681,14 @@ static const char *renderer_state_repr (renderer_state_t st

typedef struct compositor_stm_t compositor_stm_t;

static gboolean compositor_stm_linger_timeout_cb (gpointer aptr);
static void compositor_stm_cancel_linger_timeout (compositor_stm_t *self);
static void compositor_stm_schedule_linger_timeout (compositor_stm_t *self);

static void compositor_stm_lingerer_info_cb (const peerinfo_t *peerinfo, gpointer aptr);
static const char *compositor_stm_get_lingerer (const compositor_stm_t *self);
static void compositor_stm_set_lingerer (compositor_stm_t *self, const char *name);

static void compositor_stm_ctor (compositor_stm_t *self);
static void compositor_stm_dtor (compositor_stm_t *self);

Expand Down Expand Up @@ -5972,6 +5980,23 @@ struct compositor_stm_t
*/
gchar *csi_service_owner;

/** Private name of the previous compositor D-Bus service owner
*
* Access via:
* compositor_stm_get_lingerer()
* compositor_stm_set_lingerer()
*/
gchar *csi_lingering_owner;

/** Timer id for ignoring previous compositor D-Bus service owner
*
* Used by:
* compositor_stm_schedule_linger_timeout()
* compositor_stm_cancel_linger_timeout()
* compositor_stm_linger_timeout_cb()
*/
guint csi_linger_timeout_id;

/** Process identifier of the compositor D-Bus service
*
* Modify via compositor_stm_set_service_pid()
Expand Down Expand Up @@ -6059,6 +6084,10 @@ compositor_stm_ctor(compositor_stm_t *self)
self->csi_service_owner = 0;
self->csi_service_pid = COMPOSITOR_STM_INVALID_PID;

/* There is no lingering previous name owner */
self->csi_lingering_owner = 0;
self->csi_linger_timeout_id = 0;

/* On startup we want to enable compositor asap */
self->csi_target = RENDERER_ENABLED;
self->csi_requested = RENDERER_UNKNOWN;
Expand Down Expand Up @@ -6647,6 +6676,122 @@ compositor_stm_bury_timer_cb(void *aptr)
return FALSE;
}

/* ------------------------------------------------------------------------- *
* manage waiting for previous compositor name owner to exit
* ------------------------------------------------------------------------- */

/** Timer callback for: lingering compositor timeout
*
* Stop waiting for the previous compositor to exit and
* and yield control to the current one.
*/
static gboolean
compositor_stm_linger_timeout_cb(gpointer aptr)
{
compositor_stm_t *self = aptr;

self->csi_linger_timeout_id = 0;

mce_log(LL_DEBUG, "linger timeout triggered");

// forget compositor we have done ipc with
compositor_stm_set_lingerer(self, 0);

return G_SOURCE_REMOVE;
}

/** Cancel scheduled lingering compositor timeout
*/
static void
compositor_stm_cancel_linger_timeout(compositor_stm_t *self)
{
if( self->csi_linger_timeout_id ) {
mce_log(LL_DEBUG, "linger timeout canceled");
g_source_remove(self->csi_linger_timeout_id),
self->csi_linger_timeout_id = 0;
}
}

/** Schedule lingering compositor timeout
*/
static void
compositor_stm_schedule_linger_timeout(compositor_stm_t *self)
{
compositor_stm_cancel_linger_timeout(self);

mce_log(LL_DEBUG, "linger timeout scheduled");
self->csi_linger_timeout_id =
g_timeout_add(5000, compositor_stm_linger_timeout_cb, self);
}

/** Peerinfo notification callback for lingering compositor
*
* Once the previous compositor makes an exit (it is assumed
* that dropping out of system bus is good enough approximation)
* we can yield control to a new / waiting compositor instance.
*/
static void
compositor_stm_lingerer_info_cb(const peerinfo_t *peerinfo, gpointer aptr)
{
compositor_stm_t *self = aptr;
const char *name = peerinfo_name(peerinfo);
pid_t pid = peerinfo_get_owner_pid(peerinfo);
peerstate_t state = peerinfo_get_state(peerinfo);

mce_log(LL_DEBUG, "lingering compositor: name=%s pid=%d state=%s",
name, (int)pid, peerstate_repr(state));

if( state == PEERSTATE_STOPPED ) {
if( !g_strcmp0(compositor_stm_get_lingerer(self), name) )
compositor_stm_set_lingerer(self, 0);
}
}

/** Get private name of lingering compositor service
*/
static const char *
compositor_stm_get_lingerer(const compositor_stm_t *self)
{
return self->csi_lingering_owner;
}

/** Set private name of lingering compositor service
*
* Should be called as soon as a compositor instance has been
* accepted as target of compositor dbus ipc.
*/
static void
compositor_stm_set_lingerer(compositor_stm_t *self, const char *name)
{
if( !g_strcmp0(self->csi_lingering_owner, name) )
goto EXIT;

if( self->csi_lingering_owner ) {
mce_log(LL_DEBUG, "lingering compositor: name=%s - ignored",
self->csi_lingering_owner);
mce_dbus_name_tracker_remove(self->csi_lingering_owner,
compositor_stm_lingerer_info_cb, self);
g_free(self->csi_lingering_owner),
self->csi_lingering_owner = 0;

compositor_stm_cancel_linger_timeout(self);
}

if( name ) {
self->csi_lingering_owner = g_strdup(name);

mce_log(LL_DEBUG, "lingering compositor: name=%s - tracked",
self->csi_lingering_owner);
mce_dbus_name_tracker_add(self->csi_lingering_owner,
compositor_stm_lingerer_info_cb, self, 0);
}

compositor_stm_eval_state(self);

EXIT:
return;
}

/* ------------------------------------------------------------------------- *
* compositor ipc state machine
* ------------------------------------------------------------------------- */
Expand Down Expand Up @@ -6712,6 +6857,9 @@ compositor_stm_enter_state(compositor_stm_t *self)
// deactivate all led patterns
for( compositor_led_t led = 0; led < COMPOSITOR_LED_NUMOF; ++led )
compositor_led_set_active(led, false);

// clear previous compositor tracking data
compositor_stm_set_lingerer(self, 0);
break;

case COMPOSITOR_STATE_STOPPED:
Expand All @@ -6721,13 +6869,19 @@ compositor_stm_enter_state(compositor_stm_t *self)
break;

case COMPOSITOR_STATE_STARTED:
// apply timeout for waiting previous compositor to exit
if( compositor_stm_get_lingerer(self) )
compositor_stm_schedule_linger_timeout(self);

self->csi_panic_delay = COMPOSITOR_STM_INITIAL_PANIC_DELAY;

compositor_stm_send_pid_query(self);
compositor_stm_set_state(self, COMPOSITOR_STATE_REQUESTING);
break;

case COMPOSITOR_STATE_REQUESTING:
// remember compositor we have done ipc with
compositor_stm_set_lingerer(self, self->csi_service_owner);

compositor_stm_set_requested(self, self->csi_target);
compositor_stm_schedule_killer(self);
compositor_stm_schedule_panic(self);
Expand Down Expand Up @@ -6778,6 +6932,7 @@ compositor_stm_leave_state(compositor_stm_t *self)
break;

case COMPOSITOR_STATE_STARTED:
compositor_stm_cancel_linger_timeout(self);
break;

case COMPOSITOR_STATE_REQUESTING:
Expand Down Expand Up @@ -6815,6 +6970,9 @@ compositor_stm_eval_state(compositor_stm_t *self)
break;

case COMPOSITOR_STATE_STARTED:
/* Proceed once lingering compositor has exited / is ignored */
if( !compositor_stm_get_lingerer(self) )
compositor_stm_set_state(self, COMPOSITOR_STATE_REQUESTING);
break;

case COMPOSITOR_STATE_REQUESTING:
Expand Down

0 comments on commit d42285d

Please sign in to comment.