also use xcb reference counting underneath Display connection functions to ensure matching connection object lifetimes
Commit by Josh Triplett and Jamey Sharp. Signed-off-by: Josh Triplett <[email protected]> Signed-off-by: Jamey Sharp <[email protected]> Signed-off-by: Mike Blumenkrantz <[email protected]> --- include/X11/Xlib-xcb.h | 1 + man/Makefile.am | 1 + man/XOpenDisplay.man | 10 +++- man/XOpenXCBConnection.man | 40 +++++++++++++ src/ClDisplay.c | 8 +++ src/OpenDis.c | 144 ++++++++++++++++++++++++++++++++------------- src/Xxcbint.h | 3 + 7 files changed, 163 insertions(+), 44 deletions(-) create mode 100644 man/XOpenXCBConnection.man diff --git a/include/X11/Xlib-xcb.h b/include/X11/Xlib-xcb.h index a21e2be..0ee8b47 100644 --- a/include/X11/Xlib-xcb.h +++ b/include/X11/Xlib-xcb.h @@ -11,6 +11,7 @@ _XFUNCPROTOBEGIN xcb_connection_t *XGetXCBConnection(Display *dpy); +Display *XOpenXCBConnection(xcb_connection_t *c); enum XEventQueueOwner { XlibOwnsEventQueue = 0, XCBOwnsEventQueue }; void XSetEventQueueOwner(Display *dpy, enum XEventQueueOwner owner); diff --git a/man/Makefile.am b/man/Makefile.am index ef1a745..ad54c93 100644 --- a/man/Makefile.am +++ b/man/Makefile.am @@ -118,6 +118,7 @@ libman_PRE = \ XNextEvent.man \ XNoOp.man \ XOpenDisplay.man \ + XOpenXCBConnection.man \ XOpenIM.man \ XOpenOM.man \ XParseGeometry.man \ diff --git a/man/XOpenDisplay.man b/man/XOpenDisplay.man index 30859b2..07abd38 100644 --- a/man/XOpenDisplay.man +++ b/man/XOpenDisplay.man @@ -131,7 +131,8 @@ returns a pointer to a .ZN Display structure, which is defined in -.hN X11/Xlib.h . +.hN X11/Xlib.h . The underlying connection object is reference counted, +and it will have a count of 1 upon returning from this call. If .ZN XOpenDisplay does not succeed, it returns NULL. @@ -157,7 +158,8 @@ see section 2.2.1. .LP The .ZN XCloseDisplay -function closes the connection to the X server for the display specified in the +function decrements the reference count of the underlying connection. +If the count reaches 0 it closes the connection to the X server for the display specified in the .ZN Display structure and destroys all windows, resource IDs .Pn ( Window , @@ -171,7 +173,9 @@ or other resources that the client has created on this display, unless the close-down mode of the resource has been changed (see .ZN XSetCloseDownMode ). -Therefore, these windows, resource IDs, and other resources should never be +Therefore, if +.ZN XCloseDisplay +returns 0 these windows, resource IDs, and other resources should never be referenced again or an error will be generated. Before exiting, you should call .ZN XCloseDisplay diff --git a/man/XOpenXCBConnection.man b/man/XOpenXCBConnection.man new file mode 100644 index 0000000..bbb9fc5 --- /dev/null +++ b/man/XOpenXCBConnection.man @@ -0,0 +1,40 @@ +.\" Copyright \(co 2010 Josh Triplett, Jamey Sharp +.\" +.\" Permission is hereby granted, free of charge, to any person obtaining +.\" a copy of this software and associated documentation files (the +.\" "Software"), to deal in the Software without restriction, including +.\" without limitation the rights to use, copy, modify, merge, publish, +.\" distribute, sublicense, and/or sell copies of the Software, and to +.\" permit persons to whom the Software is furnished to do so, subject to +.\" the following conditions: +.\" +.\" The above copyright notice and this permission notice shall be included +.\" in all copies or substantial portions of the Software. +.\" +.\" THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS +.\" OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +.\" MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. +.\" IN NO EVENT SHALL THE X CONSORTIUM BE LIABLE FOR ANY CLAIM, DAMAGES OR +.\" OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, +.\" ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR +.\" OTHER DEALINGS IN THE SOFTWARE. +.\" +.TH XGetXCBConnection __libmansuffix__ __xorgversion__ "XLIB FUNCTIONS" +.SH NAME +XOpenXCBConnection \- create an Xlib Display for an existing XCB connection +.SH SYNTAX +.HP +#include <X11/Xlib-xcb.h> +.HP +Display *XOpenXCBConnection(xcb_connection_t *\fIc\fP); +.SH ARGUMENTS +.IP \fIc\fP 1i +Specifies the xcb_connection_t to open a Display for. +.SH DESCRIPTION +The \fIXOpenXCBConnection\fP function returns the Xlib Display associated with +an XCB connection, creating that Display if none exists. The connection object +is reference counted and will only be deleted when all references are removed. +.SH "SEE ALSO" +XOpenDisplay(__libmansuffix__), +.br +\fIXlib \- C Language X Interface\fP diff --git a/src/ClDisplay.c b/src/ClDisplay.c index bddd773..3ad2d89 100644 --- a/src/ClDisplay.c +++ b/src/ClDisplay.c @@ -48,6 +48,12 @@ XCloseDisplay ( register _XExtension *ext; register int i; + if (dpy->xcb->refcount > 1) { + _XLockMutex(_Xglobal_lock); + dpy->xcb->refcount--; + _XUnlockMutex(_Xglobal_lock); + return 1; + } if (!(dpy->flags & XlibDisplayClosing)) { dpy->flags |= XlibDisplayClosing; @@ -69,6 +75,8 @@ XCloseDisplay ( XSync(dpy, 1); } xcb_disconnect(dpy->xcb->connection); + _XLockMutex(_Xglobal_lock); _XFreeDisplayStructure (dpy); + _XUnlockMutex(_Xglobal_lock); return 0; } diff --git a/src/OpenDis.c b/src/OpenDis.c index 509060e..d62cf14 100644 --- a/src/OpenDis.c +++ b/src/OpenDis.c @@ -54,6 +54,8 @@ static xReq _dummy_request = { 0, 0, 0 }; +static Display *dpy_head; + static void OutOfMemory(Display *dpy); /* @@ -66,11 +68,59 @@ XOpenDisplay ( { xcb_connection_t *c; register Display *dpy; /* New Display object being created. */ + char *display_name; /* pointer to display name */ + int iscreen; /* screen number */ + /* + * If the display specifier string supplied as an argument to this + * routine is NULL or a pointer to NULL, read the DISPLAY variable. + */ + if (display == NULL || *display == '\0') { + if ((display_name = getenv("DISPLAY")) == NULL) { + /* Oops! No DISPLAY environment variable - error. */ + return(NULL); + } + } + else { + /* Display is non-NULL, copy the pointer */ + display_name = (char *)display; + } + +/* + * Call the Connect routine to get the transport connection object. + * If NULL is returned, the connection failed. + */ + if(!(c = _XConnectXCB(display, &iscreen))) { + return NULL; + } + + dpy = XOpenXCBConnection(c); + /* Drop the reference taken by XOpenXCBConnection; Xlib owns this + * connection. */ + xcb_disconnect(c); + + if ((dpy->display_name = strdup(display_name)) == NULL) { + OutOfMemory(dpy); + return(NULL); + } + +/* + * Make sure default screen is legal. + */ + dpy->default_screen = iscreen; /* Value returned by ConnectDisplay */ + if (iscreen >= dpy->nscreens) { + OutOfMemory(dpy); + return(NULL); + } + + return dpy; +} + +static Display *_XOpenXCBConnection(xcb_connection_t *c) +{ + Display *dpy; /* New Display object being created. */ register int i; int j, k; /* random iterator indexes */ - char *display_name; /* pointer to display name */ char *setup = NULL; /* memory allocated at startup */ - int iscreen; /* screen number */ xConnSetupPrefix prefix; /* prefix information */ int vendorlen; /* length of vendor string */ union { @@ -86,22 +136,13 @@ XOpenDisplay ( long usedbytes = 0; /* number of bytes we have processed */ unsigned long mask; long int conn_buf_size; - char *xlib_buffer_size; + char *xlib_buffer_size, *display_name; - /* - * If the display specifier string supplied as an argument to this - * routine is NULL or a pointer to NULL, read the DISPLAY variable. - */ - if (display == NULL || *display == '\0') { - if ((display_name = getenv("DISPLAY")) == NULL) { - /* Oops! No DISPLAY environment variable - error. */ - return(NULL); - } - } - else { - /* Display is non-NULL, copy the pointer */ - display_name = (char *)display; - } + for (dpy = dpy_head; dpy; dpy = dpy->xcb->dpy_next) + if (dpy->xcb->connection == c) + return dpy; + + xcb_reference(c); /* * Set the default error handlers. This allows the global variables to @@ -111,15 +152,6 @@ XOpenDisplay ( if (_XIOErrorFunction == NULL) (void) XSetIOErrorHandler (NULL); /* - * Call the Connect routine to get the transport connection object. - * If NULL is returned, the connection failed. - */ - - if(!(c = _XConnectXCB(display, &iscreen))) { - return NULL; - } - -/* * Attempt to allocate a display structure. Return NULL if allocation fails. */ if ((dpy = (Display *)Xcalloc(1, sizeof(Display) + sizeof(_X11XCBPrivate))) == NULL) { @@ -130,12 +162,6 @@ XOpenDisplay ( dpy->fd = xcb_get_file_descriptor(c); dpy->xcb = (_X11XCBPrivate *) (dpy + 1); dpy->xcb->connection = c; - - if ((dpy->display_name = strdup(display_name)) == NULL) { - OutOfMemory(dpy); - return NULL; - } - dpy->xcb->next_xid = xcb_generate_id(dpy->xcb->connection); dpy->xcb->event_notify = xcondition_malloc(); @@ -147,6 +173,19 @@ XOpenDisplay ( xcondition_init(dpy->xcb->event_notify); xcondition_init(dpy->xcb->reply_notify); + /* Not necessarily correct, but this can only go wrong if opening an + * xcb_connection_t using a display_name other than $DISPLAY, *and* + * using XOpenXCBConnection rather than XOpenDisplay, *and* then + * attempting to reconnect to dpy->display_name. */ + display_name = getenv("DISPLAY"); + if (display_name != NULL) { + dpy->display_name = strdup(display_name); + if (!dpy->display_name) { + OutOfMemory(dpy); + return NULL; + } + } + /* Initialize as much of the display structure as we can. * Initialize pointers to NULL so that XFreeDisplayStructure will * work if we run out of memory before we finish initializing. @@ -215,7 +254,6 @@ XOpenDisplay ( dpy->savedsynchandler = NULL; dpy->request = 0; dpy->last_request_read = 0; - dpy->default_screen = iscreen; /* Value returned by ConnectDisplay */ dpy->last_req = (char *)&_dummy_request; /* Initialize the display lock */ @@ -501,14 +539,6 @@ XOpenDisplay ( */ /* - * Make sure default screen is legal. - */ - if (iscreen >= dpy->nscreens) { - OutOfMemory(dpy); - return(NULL); - } - -/* * get availability of large requests */ dpy->bigreq_size = xcb_get_maximum_request_length(dpy->xcb->connection); @@ -575,12 +605,37 @@ XOpenDisplay ( #ifdef XKB XkbUseExtension(dpy,NULL,NULL); #endif + + dpy->xcb->dpy_next = dpy_head; + dpy_head = dpy; + /* * and return successfully */ return(dpy); } +/* + * Construct a Display from an existing xcb_connection_t. + */ +Display *XOpenXCBConnection(xcb_connection_t *c) +{ + Display *dpy; +/* + * Set the default error handlers here to prevent deadlocks later. + * This allows the global variables to default to NULL for use with shared libraries. + */ + if (_XErrorFunction == NULL) (void) XSetErrorHandler (NULL); + if (_XIOErrorFunction == NULL) (void) XSetIOErrorHandler (NULL); + _XLockMutex(_Xglobal_lock); + dpy = _XOpenXCBConnection(c); + if (dpy) + dpy->xcb->refcount++; + _XUnlockMutex(_Xglobal_lock); + return dpy; +} + + /* XFreeDisplayStructure frees all the storage associated with a * Display. It is used by XOpenDisplay if it runs out of memory, * and also by XCloseDisplay. It needs to check whether all pointers @@ -592,6 +647,13 @@ XOpenDisplay ( void _XFreeDisplayStructure(Display *dpy) { + Display **dpy_cur; + for (dpy_cur = &dpy_head; *dpy_cur; dpy_cur = &(*dpy_cur)->xcb->dpy_next) + if (*dpy_cur == dpy) { + *dpy_cur = dpy->xcb->dpy_next; + break; + } + /* move all cookies in the EQ to the jar, then free them. */ if (dpy->qfree) { _XQEvent *qelt = dpy->qfree; diff --git a/src/Xxcbint.h b/src/Xxcbint.h index b4f8988..f0a9e6a 100644 --- a/src/Xxcbint.h +++ b/src/Xxcbint.h @@ -39,6 +39,9 @@ typedef struct _X11XCBPrivate { xcondition_t event_notify; int event_waiter; xcondition_t reply_notify; + + int refcount; + Display *dpy_next; } _X11XCBPrivate; /* xcb_disp.c */ -- 2.4.2 _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
