[PATCH] record: Prevent a crash on recording client disconnect.

[Keith Packard]

On Tue, 22 Jun 2010 14:57:20 +0300, Rami Ylimäki <ext-rami.ylimaki at nokia.com> wrote:
> Execute the following steps to reproduce the issue.
> 
>   1. Run at least two recording clients simultaneously.
>      $ cnee --record --request-range 1-127 &
>      $ cnee --record --request-range 1-127 &
>   2. Kill the recording clients.
>      $ killall cnee
>   3. Give X server something to do so that the clients are closed.
>      $ xinput list
>      $ xinput list
> 
> As a result RecordUninstallHooks accesses NullClient, because
> RecordAClientStateChange doesn't clean the recording clients up
> properly.
> 
> Fix RecordUninstallHooks to fail locally on an assertion instead of
> much later in privates code, if NullClient is still accessed because
> of some other bug. Fix RecordAClientStateChange to iterate through all
> contexts so that modifications of the iterated array during iteration
> don't cause contexts to be skipped.
> 
> Signed-off-by: Rami Ylimäki <ext-rami.ylimaki at nokia.com>

Yikes. The amount of terrible code in the X server is daunting some
days. In this case, the cost of walking over 'disabled' contexts was
deemed so high that the array of context pointers is mashed whenever a
context is enabled or disabled, moving the set of enabled contexts to
the first part of the array while the disabled contexts live at the back
of the array.

What I didn't realize at first is that there are two counters in the
array -- numContexts and numEnabledContexts, and that when a client
disappears, it's contexts aren't deleted, they're just disabled, and so
they hang around in this array, but get moved to the end. So, copying
the array of pointers and walking it once will visit each context
precisely once, without touching any invalid pointers.

Krh may yet convince me that the generic list functions might well be a
good plan for things like this :-)

Reviewed-by: Keith Packard <keithp at keithp.com>

-- 
keith.packard at intel.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.x.org/archives/xorg-devel/attachments/20100622/a2f7cde1/attachment.pgp>