Flaws in DockApps
Most of the DockApps in Debian are incorrectly written. A solution has been
found but a method to implement the solution is still to be found.
1. Code
Lots of DockApps are written like this:
1: #define DELAY 10000L
2:
3: int main (int argc, char **argv) {
4: XEvent event;
...
5: /* initalise dockapp */
...
6: /* main event loop */
7: while(1) {
8: /* process any pending X events */
9: while (XPending(display)) {
10: XNextEvent(display, &event):
11: switch(event.type) {
12: case Expose:
13: RedrawWindow();
14: break;
15: case ButtonPress:
...
16: break;
17: /* more cases */
...
18: }
19: }
20:
21: /* collect data */
...
22: RedrawWindow();
23:
24: usleep(DELAY);
25: }
26: }
2. Analysis
IMHO it is incorrect to use a usleep (of this amount of time) and an
XNextEvent in the same loop. In the worst case the DockApp has to wait DELAY
microsecond from being sent an XEvent, until it has a chance to handle it. An
Exposure event (when a window covering the DockApp has been moved to expose
the DockApps) causes white patches that are visible on the DockApp during the
time it is asleep. This looks ugly and should be correct.
As a result of the usleep there are other problems in the code.
Double redraw
1. the DockApp is sent an Exposure event while it is asleep (line 24)
2. it waits until usleep is complete (line 25)
3. handles the Exposure event by redrawing the DockApp (lines 9-14)
4. collects the data that it displays (lines 21-...)
5. and redraws the DockApp again (22)
The DockApp is being redraw twice, when there is no need to redraw it twice.
Redraw event when not needed
The DockApp is being redraw once every second, the data it is displaying might
not have changed, DockApps should be redrawn only if they need to be. There
are two reasons for redrawing a DockApp, a change in the display or an
Exposure XEvent.
3. Solution
If the usleep (line 22) is removed, the DockApp will not have to wait DELAY
microseconds before responding to an XEvent, but it will use 100% CPU time
checking for events and updating its display.
If the usleep (line 22) and while(XPending(display)) (line 8) are removed,
while the XNextEvent (line 9) and switch(event.type) (line 10) are moved to
the main event loop, the DockApp will block on XNextEvent, it will wait for an
event to be ready for processing, instead of waiting on the usleep. The
DockApp is redrawn immediately after receiving an event, but on the other
hand it is *only* redrawn immediately after receiving an event. If the
DockApp goes for 5 hours without receiving an event it will not redraw itself.
Each time an XEvent is handled the data that the DockApp shows will be
recollected, lots of XEvents mean data is collected more often than needed.
There needs to be some way to block waiting for an XEvent, but with a time-out
of DELAY microseconds. A function that blocks for at most DELAY microseconds,
but can return earlier it if the DockApp receives an XEvent.
The solution, the select library call, was found by reading the libdockapp
source code. It is described in the man as providing `synchronous I/O
multiplexing'.
Here is the prototype:
int select(int n, fd_set *readfds, fd_set *writefds,
fd_set *exceptfds, struct timeval *timeout);
Select will check to see if any file descriptor has data ready to read or
is ready to be written to, with a timeout. The file descriptor for a specific
X display is provided by the ConnectionNumber function.
4. Sample implementation
An implementation using the select library call:
1: #define DELAY 10000L
2:
3: int main (int argc, char **argv) {
4: XEvent event;
5: fd_set fds;
6: struct timeval tv;
7: int fd, int data_changed;
...
8: /* initialise dockapp */
...
9: displayfd = ConnectionNumber(display)
10: FD_ZERO(&fds);
11: FD_SET(displayfd, &fds);
12: tv.tv_sec = DELAY / 10000L
13: tv.tv_usec = DELAY % 10000L
14:
15: /* main event loop */
16: while(1) {
17: /* collect data (sets data_changed as needed) */
...
18: if (data_changed) Redraw_Window();
19:
20: /* handle X events */
21: while(select(displayfd + 1, NULL, NULL, &tv)) {
22: XNextEvent(display, &event):
23: switch(event.type) {
...
24: }
25: }
26: }
27: }
5. Method of implementation
The problem has been found, analysed, and a solution founds. All that remains
to be found is a method to implement this solution. Lots of the DockApp code
is very similar, it is good programming practice to keep duplicate code to a
minimum, but it is good Debian practice to avoid rewriting upstream code, and
concentrate on packaging. There is no doubt that we should try and fix these
bugs, because they are clearly bugs, but how should we do it?
E-mail upstream maintainers, explain the problem, and ask them to implement
the solution. Less coding for Debian, but we should expect inconsistent
results.
Write patches for each DockApp and file them to the BTS, ask DockApp
maintainers to apply the patches and forward them upstream. More work for
Debian, but more consistent results.
Rewrite all the DockApps to use libdockapp (or similar). Would mean less code
in Debian, with all the advantages that removing code duplication brings.
Would be nice, but not really what Debian is about, we are just meant to
package things. Upstream maintainers would probably not be best pleased with
us rewriting there programs.
Although lots of the code is similar, some is different, for example while
a number of the DockApps require a command line switch to be run in Docked
form, others do not. A rewrite would help us to make the DockApps more
consistent. On the other hand the fact that the code is similar in lots of
places and different in others makes a rewrite even harder.
6. Final thought
The problem has already been fixed. Take a look at the epplets package, lots
of DockApp like programs, they do not have the problem described above,
and are written with a consistent user interface. Problem is they only work
with Enlightenment.
Alternative solution to the DockApp problem would involve deprecating the
DockApps in favour of Epplets. An Epplet->DockApp wrapper would need to be
written, so that Epplets can be used with Window Managers that support
DockApps, or window managers could be modified to support Epplets natively.
First steps have been taken towards more wide spread Epplet support, but
development is stalled because the interface between Enlightenment and the
Epplets is more complex than the interface between window managers and
DockApps.
--
Edward Betts (GPG: 1BC4E32B)
Reply to: