Proposed libX11 ABI break

[Peter Hutterer]

This week, I ran into a wall trying to get XI2 events sorted out. Resolving
this may require a libX11 ABI break. This is NOT XI2 specific, it just
happens to be the first extension to run into this issue.
This is a rather lengthy email, please take the time and read it.

== Problem ==
XNextEvent(Display*, XEvent*) requires the event to be 96 (32-bit) or 192
(64-bit) bytes. This is a side-effect of the definition  of the XEvent
structure itself.

XGE's long events may require structs that exceed this space. For XI2, the
solution I have used so far are pointers inside the respective XEvent struct
that point to other allocated memory. The client is required to call
XIFreeEventData() to ensure this extra memory is freed accordingly.

Example of correct use:
    XIEvent xi2ev;
    XNextEvent(dpy, &xi2ev);
    XIFreeEventData(&xi2ev); 

This introduces a severe memory leak in client applications. For example,
the following snippet will leak with most generic event received.

while (1) { 
  XNextEvent(dpy, &event); 
  if (event.type == MapNotify)
      break;
}

The above event loop could be fixed up by adding the XIFreeEventData call.
However, in the current approach, the *Free* functions are extension
specific, requiring a *Free* call for each extension that is provides XGE
events.

== Solution ==
keithp came up with an approach to avoid this issue: libX11 can keep track of 
such events and return a cookie to the client. The client then calls an
extra function XGEGetEventData(cookie) and receive the actual data for the
event. At this point, it is the client's responsibility to free the data.
Unclaimed cookies are freed with the next call to XNextEvent, ensuring the
above loop does not leak. This can be done in a generic,
extension-independent manner.

A side-effect of this is also that this decouples the actual event data from
the XEvent struct for all new events. These events can thus use arbitrary
representations of data, including data sizes that are the same across
architectures. The event returned to the user is merely a stub to contain
the cookie.

Example code for this approach:
  XEvent event;
  XIEvent *xi2ev;
  XNextEvent(dpy, &event);

  XNextEvent(dpy, &event); /* first event cleaned up automatically */
  xi2ev = XGEGetEvent(dpy, &event); /* we are now responsible for freeing */
  XGEFreeEvent(xi2ev);
  
The simplest implementation for the above approach is to add two fields, the
cookie and the data pointer, to the XGenericEvent definition and require
extensions to only fill in those when retrieving the event from the wire.
The actual event data is stored separately in memory.

typedef struct
{
    int            type;
    unsigned long  serial;
    Bool           send_event;
    Display        *display;
    int            extension;
    int            evtype;
    unsigned long  cookie;      /* new field */
    void           *data;       /* new field */
} XGenericEvent;

The advantage of this solution is that XNextEvent, XPutBackEvent and friends
can be made to work with this new cookie + data approach.

Albeit, the XGenericEvent structure has been part of the libX11-1.2 release.
Adding fields to it constitutes an libX11 ABI break. Furthermore, the
requirement for extensions to only fill cookie + data is a significant
behavioural change. As a result, any current users of XGE will fail in
unexpected ways when running against the next libX11 release.

To partially mitigate this, the XGenericEvent struct could be renamed into
XEventCookie or somesuch. Clients that use XGenericEvents would then see an
API change when recompiled against the new library, reducing the impact a
bit.

FWIW, I am not aware of any users of GenericEvents aside from the unreleased
XI2 and the protocol part of it is unaffected. This is Xlib only.

== Alternative Solution ==
The only other solution I could come up with so far is to add XGENextEvent()
and friends as substitutes for XNextEvent & co. In this approach, XNextEvent
_never_ returns generic events, leaving existing clients ABI-safe.
XGENextEvent requires an argument of the cookie+data type.

This approach obviously has a high cost to clients as they need to adjust
for this new API.

[Note that some new API is unavoidable. For example, XCheckTypedEvent does
not work in light of generic events as they share the same type. Additional
XGECheckTypedEvent() APIs are required, but they are required either way.]

== Summary ==
So, to sum up, the two approaches I could come up with can be simplified as:

* ABI break
  + cost to clients low
  - ABI break

* API replacement
  + no ABI break
  - cost to clients high

I'd prefer the former, especially in the light of the limited number of
current users.

Comments?

Cheers,
  Peter