Created attachment 135061 [details] [review]
move dbus init/fini calls

The Xorg server 1.17.4 (from OL 6.9: xorg-x11-server-Xorg-1.17.4-16.0.1.el6.x86_64) goes to 100% CPU usage under certain conditions.

In my environment the X server is started on boot via a script and then a full-screen client is added.
The X server seems to restart a few times (serverGeneration is 3) until it comes up. This happens only on boot; if the X server is restarted or started later then serverGeneration is 1.

- Function dix/main.c:dix_main() among other things calls InitOutput(...).

- Function hw/xfree86/common/xf86Init.c:InitOutput() among other things calls dbus_core_init(), but only if serverGeneration is 1.

- Function config/dbus-core.c:dbus_core_init() triggers these actions:
  - Call of AddGeneralSocket to register a socket for the poll loop (see os/WaitFor.c:WaitForSomething())
  - Call of RegisterBlockAndWakeupHandlers to register a callback if some socket needs attention

- When the loop in dix_main increases the serverGeneration it calls (among many other) dix/dixutils.c:InitBlockAndWakeupHandlers(). This removes all registered callbacks but does not remove the socket added by dbus init.

- When there is activity on the dbus socket WaitForSomething() calls every registered wakeup handler, but the dbus handler is no longer there and the socket is not handled.

- On the next attempt to poll the socket, poll returns immediately and the socket is still not handled.

- The Xorg server is now in a 100% CPU loop in WaitForSomething and mostly calling wakeup handlers (in vain).

This regression was introduced by the commit:

commit 480590b90
Author: Hans de Goede <hdegoede@redhat.com>
Date:   Wed Dec 4 11:10:06 2013 +0100

    dbus-core: Make dbus-core no longer mutually exclusive with udev
    
    With systemd-logind the dbus-core will be used for more then just config, so
    it should be possible to build it even when using a non dbus dependent config
    backend.
    
    This patch also removes the config_ prefix from the dbus-core symbols.
    
    Signed-off-by: Hans de Goede <hdegoede@redhat.com>
    Reviewed-by: Daniel Stone <daniel@fooishbar.org>


which moved the dbus init/fini calls.
The old code called init/fini on every serverGeneration; now it is only called on the first serverGeneration, but destroyed by InitBlockAndWakeupHandlers.

The attached patch moves init above the if serverGeneration==1 branch and moves fini back to CloseInput.

In the current Xorg server (since commit bf920b239) dbus uses the new infrastructure "NotifyFd" which probably doesn't have this issue above -- but I didn't check.

Do you think the attached patch fixes my issue without breaking other functionality? I only tested on OL6.9 (which uses upstart and not systemd).