summaryrefslogtreecommitdiff
path: root/src/XlibInt.c
Commit message (Collapse)AuthorAgeFilesLines
* Don't leave dangling pointers in Free functionsAlan Coopersmith2020-11-181-2/+1
| | | | | | | | | | | | | | While these are mostly called during teardown of larger structures that are about to themselves be freed, there's no guarantee that will always be the case, so try to be safer here. [ This bug was found by the Parfait 4.0 bug checking tool. http://labs.oracle.com/pls/apex/f?p=labs:49:::::P49_PROJECT_ID:13 ] v2: Deduplicate & simplify pointer clearing in _XFreeEventCookies as suggested by @keithp Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
* Avoid recursing through _XError due to sequence adjustmentKeith Packard2020-11-151-3/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This patch is based on research done by Dmitry Osipenko to uncover the cause of a large class of Xlib lockups. _XError must unlock and re-lock the display around the call to the user error handler function. When re-locking the display, two functions are called to ensure that the display is ready to generate a request: _XIDHandler(dpy); _XSeqSyncFunction(dpy); The first ensures that there is at least one XID available to use (possibly calling _xcb_generate_id to do so). The second makes sure a reply is received at least every 65535 requests to keep sequence numbers in sync (possibly generating a GetInputFocus request and synchronously awaiting the reply). If the second of these does generate a GetInputFocus request and wait for the reply, then a pending error will cause recursion into _XError, which deadlocks the display. One seemingly easy fix is to have _XError avoid those calls by invoking InternalLockDisplay instead of LockDisplay. That function does everything that LockDisplay does *except* call those final two functions which may end up receiving an error. However, that doesn't protect the system from applications which call some legal Xlib function from within their error handler. Any Xlib function which cannot generate protocol or wait for events is valid, including many which invoke LockDisplay. What we need to do is make LockDisplay skip these two function calls precisely when it is called from within the _XError context for the same display. This patch accomplishes this by creating a list of threads in the display which are in _XError, and then having LockDisplay check the current thread against those list elements. Inspired-by: Dmitry Osipenko <digetx@gmail.com> Signed-off-by: Keith Packard <keithp@keithp.com> Tested-by: Dmitry Osipenko <digetx@gmail.com> Reviewed-by: Dmitry Osipenko <digetx@gmail.com>
* Add XSetIOErrorExitHandler() functionCarlos Garnacho2020-10-151-2/+16
| | | | | | | | | | | | | | | | | This function complements XSetIOErrorHandler(), allowing to override the default behavior that trusts on I/O errors never coming back (i.e. exit()ing the process). This is meant as a mechanism for Wayland compositors (that are too a X11 client + compositing manager) to unfasten seatbelts and jump through the car window. It might get lucky and land on a stack of pillows. In consequence, some functions labeled as _X_NORETURN can as a matter of fact return. So those hints were removed. Signed-off-by: Carlos Garnacho <carlosg@gnome.org> Reviewed-by: Michel Dänzer <mdaenzer@redhat.com>
* Prepare for _XIOError() possibly returningCarlos Garnacho2020-10-151-0/+2
| | | | | | | | Ensure current state is cut short on _XIOError(), possible reentrancy should be skipped through the XlibDisplayIOError flag checks. Signed-off-by: Carlos Garnacho <carlosg@gnome.org> Reviewed-by: Michel Dänzer <mdaenzer@redhat.com>
* Avoid the use of "register" keyword in public headers.Maya Rashish2020-08-281-2/+2
| | | | This causes issues when compiling code for C++17.
* Fix spelling/wording issuesAlan Coopersmith2020-07-221-1/+1
| | | | | | | Found by using: codespell --builtin clear,rare,usage,informal,code,names Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
* reduce gcc-normal warnings using casts (no object change)Thomas E. Dickey2020-04-211-3/+3
| | | | Signed-off-by: Thomas E. Dickey <dickey@invisible-island.net>
* Fix lockup in _XReply() caused by recursive synchronizationDmitry Osipenko2019-10-091-1/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | This patch is based on a suggestion made by Uli Schlachter in a comment to the bug report https://gitlab.freedesktop.org/xorg/lib/libx11/issues/93. Explanation of the bug (given by Uli Schlachter as well): An error was received and handled. Since there was an error callback set, Xlib unlocks the display, runs the error callback, and then locks the display again. This goes through _XLockDisplay and then calls _XSeqSyncFunction. On this "lock the thing"-path, Xlib notices that sequence numbers are close to wrap-around and tries to send a GetInputFocus request. However, the earlier calls already registered themselves as "we are handling replies/errors, do not interfere!" and so the code here waits for "that other thread" to be done before it continues. Only that there is no other thread, but it is this thread itself and thus a deadlock follows. The bug is relatively easy to reproduce on any desktop environment by using actively a touchscreen input that supports multitouch, i.e. practically all mobile devices are affected. Fixes: https://gitlab.freedesktop.org/xorg/lib/libx11/issues/93 Suggested-by: Uli Schlachter <psychon@znc.in> Tested-by: Dmitry Osipenko <digetx@gmail.com> Reported-by: Dmitry Osipenko <digetx@gmail.com> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
* Add autoconf checks for headers we include for FIONREADJon Turney2019-04-301-0/+7
| | | | | | Add autoconf checks for the extra headers we include to define FIONREAD. This needs sys/socket.h on Cygwin, and none of the alternatives on Windows.
* XlibInt.c: include headers needed for ioctl(...FIONREAD...) on SolarisAlan Coopersmith2019-02-231-0/+10
| | | | | | Fixes: commit 5538b3e4ae6dee Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
* _XDefaultIOError: Do better at detecting explicit shutdownAdam Jackson2019-01-161-1/+28
| | | | | | | | | | | | | | | Currently, when the X server crashes or a client is disconnected with XKillClient, you get a somewhat confusing error message from libX11 along the lines of: XIO: fatal IO error 11 (Resource temporarily unavailable) on X server ":0" after 98 requests (40 known processed) with 0 events remaining. What's happening here is the previous recvmsg has thrown EAGAIN, since the socket is non-blocking. In this case, check whether the socket has any more data to read, and if not treat it like EPIPE. Signed-off-by: Adam Jackson <ajax@redhat.com>
* _XDefaultIOError: Reformat to be less uglyAdam Jackson2019-01-161-13/+13
| | | | | Signed-off-by: Adam Jackson <ajax@redhat.com> Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
* Convert main src directory to use reallocarray()Alan Coopersmith2018-12-081-4/+5
|
* Fixes: warning: variable 'req' set but not,usedwalter harms2017-08-201-1/+1
| | | | | | | | | | Fixes: warning: variable 'req' set but not used [-Wunused-but-set-variable] by marking req _X_UNUSED Solution was discussed on xorg-devel ML Peter Hutter, Alan Coopersmith Re: [PATCH libX11 3/5] fix: warning: pointer targets in passing argument 2 of '_XSend' differ in signedness [-Wpointer-sign] Signed-off-by: harms wharms@bfs.de
* mark _XDefaultIOError as no_returnwalter harms2017-08-201-1/+1
| | | | | | mark _XDefaultIOError as no_return. No one comes back from exit() ... Signed-off-by: harms wharms@bfs.de
* _XDefaultError: set XlibDisplayIOError flag before calling exitArthur Huillet2017-03-071-0/+10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | _XReply isn't reentrant, and it can lead to deadlocks when the default error handler is called: _XDefaultError calls exit(1). It is called indirectly by _XReply when a X protocol error comes in that isn't filtered/handled by an extension or the application. This means that if the application (or one of its loaded shared libraries such as the NVIDIA OpenGL driver) has registered any _fini destructor, _fini will get called while still on the call stack of _XReply. If the destructor interacts with the X server and calls _XReply, it will hit a deadlock, looping on the following in _XReply: ConditionWait(dpy, dpy->xcb->reply_notify); It is legal for an application to make Xlib calls during _fini, and that is useful for an OpenGL driver to avoid resource leaks on the X server side, for example in the dlopen/dlclose case. However, the driver can not readily tell whether its _fini is being called because Xlib called exit, or for another reason (dlclose), so it is hard to cleanly work around this issue in the driver. This change makes it so _XReply effectively becomes a no-op when called after _XDefaultError was called, as though an XIOError had happened. The dpy connection isn't broken at that point, but any call to _XReply is going to hang. This is a bit of a kludge, because the more correct solution would be to make _XReply reentrant, maybe by broadcasting the reply_notify condition before calling the default error handler. However, such a change would carry a grater risk of introducing regressions in Xlib. This change will drop some valid requests on the floor, but this should not matter, as it will only do so in the case where the application is dying: X will clean up after it once exit() is done running. There is the case of XSetCloseDownMode(RETAIN_PERMANENT), but an application using that and wishing to clean up resources in _fini would currently be hitting a deadlock, which is hardly a better situation. Signed-off-by: Aaron Plattner <aplattner@nvidia.com> Reviewed-by: Jamey Sharp <jamey@minilop.net>
* fix for Xlib 32-bit request number issuesChristian Linhart2015-09-211-13/+18
| | | | | | | | | | Make use of the new 64-bit sequence number API in XCB 1.11.1 to avoid the 32-bit sequence number wrap in libX11. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=71338 Signed-off-by: Christian Linhart <chris@demorecorder.com> Signed-off-by: Olivier Fourdan <ofourdan@redhat.com> Reviewed-by: Adam Jackson <ajax@redhat.com>
* Do not return() after exit().Thomas Klausner2015-07-191-2/+2
| | | | | Signed-off-by: Thomas Klausner <wiz@NetBSD.org> Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
* Remove redundant null checks before freewalter harms2014-06-061-6/+6
| | | | | | | | | | This patch removes some redundant null checks before free. It should not change the code otherwise. Be aware that this is only the first series. Signed-off-by: Harms <wharms@bfs,de> Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com> Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
* Remove unused ETEST & ESZTEST macros from XlibInt.cAlan Coopersmith2014-01-051-33/+0
| | | | | | | Left behind when 15e5eaf62897 removed support for building without XCB. Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> Reviewed-by: Jasper St. Pierre <jstpierre@mecheye.net>
* libX11: check size of GetReqExtra after XFlushKees Cook2013-07-221-0/+8
| | | | | | | | | | | | | | | | | | | | | | Two users of GetReqExtra pass arbitrarily sized allocations from the caller (ModMap and Host). Adjust _XGetRequest() (called by the GetReqExtra macro) to double-check the requested length and invalidate "req" when this happens. Users of GetReqExtra passing lengths greater than the Xlib buffer size (normally 16K) must check "req" and fail gracefully instead of crashing. Any callers of GetReqExtra that do not check "req" for NULL will experience this change, in the pathological case, as a NULL dereference instead of a buffer overflow. This is an improvement, but the documentation for GetReqExtra has been updated to reflect the need to check the value of "req" after the call. Bug that manifested the problem: https://bugs.launchpad.net/ubuntu/+source/x11-xserver-utils/+bug/792628 Signed-off-by: Kees Cook <kees@outflux.net> Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com> Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
* Check for symbol existence with #ifdef, not #ifThomas Klausner2013-07-081-1/+1
| | | | | | Reviewed-by: Jamey Sharp <jamey@minilop.net> Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
* Remove more unnecessary casts from Xmalloc/calloc callsAlan Coopersmith2013-05-091-11/+9
| | | | Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
* unifdef WORD64Alan Coopersmith2013-03-031-236/+0
| | | | | | | | | | | | | | | | | | | WORD64 seems to have only been defined in <X11/Xmd.h> when building for CRAY, to handle int being a 64-bit value (ILP64, not LP64) and having 64-bit alignment requirements. It hadn't been fully supported even before autotooling, as can be seen by removed code such as: #ifdef WORD64 _XkbWriteCopyData32 Not Implemented Yet for sizeof(int)==8 #endif (mostly performed with unifdef, followed by some manual cleanup of the remaining code) Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com> Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net>
* Convert more sprintf calls to snprintfAlan Coopersmith2013-02-161-4/+4
| | | | | | | | You could analyze most of these and quickly recognize that there was no chance of buffer overflow already, but why make everyone spend time doing that when we can just make it obviously safe? Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
* Preserve constness in casting arguments through the Data*() routinesAlan Coopersmith2013-02-161-6/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | Casts were annoying gcc by dropping constness when changing types, when routines simply either copy data into the request buffer or send it directly to the X server, and never modify the input. Fixes gcc warnings including: ChProp.c: In function 'XChangeProperty': ChProp.c:65:6: warning: cast discards '__attribute__((const))' qualifier from pointer target type [-Wcast-qual] ChProp.c:65:6: warning: cast discards '__attribute__((const))' qualifier from pointer target type [-Wcast-qual] ChProp.c:74:6: warning: cast discards '__attribute__((const))' qualifier from pointer target type [-Wcast-qual] ChProp.c:74:6: warning: cast discards '__attribute__((const))' qualifier from pointer target type [-Wcast-qual] ChProp.c:83:6: warning: cast discards '__attribute__((const))' qualifier from pointer target type [-Wcast-qual] SetHints.c: In function 'XSetStandardProperties': SetHints.c:262:20: warning: cast discards '__attribute__((const))' qualifier from pointer target type [-Wcast-qual] SetPntMap.c: In function 'XSetPointerMapping': SetPntMap.c:46:5: warning: cast discards '__attribute__((const))' qualifier from pointer target type [-Wcast-qual] SetPntMap.c:46:5: warning: cast discards '__attribute__((const))' qualifier from pointer target type [-Wcast-qual] StBytes.c: In function 'XStoreBuffer': StBytes.c:97:33: warning: cast discards '__attribute__((const))' qualifier from pointer target type [-Wcast-qual] StName.c: In function 'XStoreName': StName.c:40:27: warning: cast discards '__attribute__((const))' qualifier from pointer target type [-Wcast-qual] StName.c: In function 'XSetIconName': StName.c:51:27: warning: cast discards '__attribute__((const))' qualifier from pointer target type [-Wcast-qual] Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
* Typo fixPeter Hutterer2012-04-301-1/+1
| | | | Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
* Add _XGetRequest as substitute for GetReq/GetReqExtraPeter Hutterer2011-11-081-0/+31
| | | | | Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net> Reviewed-by: Jamey Sharp <jamey@minilop.net>
* Call _XErrorFunction without holding the Display lock.Jamey Sharp2011-03-151-1/+13
| | | | | | | | | | | | | | | | | | | | | Historically, Xlib dropped the Display lock around the upcall to any user-supplied _XErrorFunction, but somewhere along the way I quit doing that if you built with XCB. The reasons are lost somewhere in the pre-git history of Xlib/XCB, and I can't now see any reason to hold the lock. The documentation for XSetErrorHandler still applies though: Because this condition is not assumed to be fatal, it is acceptable for your error handler to return; the returned value is ignored. However, the error handler should not call any functions (directly or indirectly) on the display that will generate protocol requests or that will look for input events. So while you are now once again permitted to re-enter Xlib from the error handler, you're only allowed to call non-protocol functions. Signed-off-by: Jamey Sharp <jamey@minilop.net>
* XlibInt: Use strncpy+zero termination instead of strcpy to enforce buffer sizeErkki Seppälä2011-02-011-3/+4
| | | | | | | | | | Possible overrun of 8192 byte fixed size buffer "buffer" by copying "ext->name" without length checking Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com> Reviewed-by: Ander Conselvan de Oliveira <ander.conselvan-de-oliveira@nokia.com> Signed-off-by: Erkki Seppälä <erkki.seppala@vincit.fi> Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
* XlibInt: info_list->watch_data was reallocated, but result was discardedAnder Conselvan de Oliveira2011-02-011-0/+1
| | | | | | | | | | | | | | info_list->watch_data was being reallocated, but the return value of the reallocation was stored only into a local variable. This might cause some funky behavior and crashes. Variable "wd_array" goes out of scope Value "wd_array" is overwritten in "wd_array = (XPointer*)realloc((char*)info_list->watch_data, (((dpy->watcher_count + 1) * 4U == 0U) ? 1U : ((dpy->watcher_count + 1) * 4U)))" Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com> Reviewed-by: Erkki Seppälä <erkki.seppala@vincit.fi> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan-de-oliveira@nokia.com> Signed-off-by: Alan Coopersmith <alan.coopersmith@oracle.com>
* Using freed pointer "e"Erkki Seppälä2011-01-311-2/+2
| | | | | | | | Reordered code to first to do the comparison and then to release data Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com> Reviewed-by: Ander Conselvan de Oliveira <ander.conselvan-de-oliveira@nokia.com> Signed-off-by: Erkki Seppälä <erkki.seppala@vincit.fi>
* Initialize event typePauli Nieminen2011-01-031-0/+2
| | | | | | | | | | | | | | | | | | | | | | | If we receive unsupported event closing connection triggers valgrind error. ==12017== Conditional jump or move depends on uninitialised value(s) ==12017== at 0x487D454: _XFreeDisplayStructure (OpenDis.c:607) ==12017== by 0x486857B: XCloseDisplay (ClDisplay.c:72) *snip* ==12017== Uninitialised value was created by a heap allocation ==12017== at 0x4834C48: malloc (vg_replace_malloc.c:236) ==12017== by 0x4894147: _XEnq (XlibInt.c:877) ==12017== by 0x4891BF3: handle_response (xcb_io.c:335) ==12017== by 0x4892263: _XReply (xcb_io.c:626) *snip* Problem is that XFreeDisplaySturture is checking for qelt->event.type == GenericEvent while _XUnknownWireEvent doesn't store the type. Reviewed-by: Adam Jackson <ajax@redhat.com> Reviewed-by: Peter Hutterer <peter.hutterer@who-t.net> Signed-off-by: Pauli Nieminen <ext-pauli.nieminen@nokia.com>
* Purge macros NEED_EVENTS and NEED_REPLIESFernando Carrijo2010-07-071-2/+0
| | | | | | Signed-off-by: Fernando Carrijo <fcarrijo@yahoo.com.br> Acked-by: Tiago Vignatti <tiago.vignatti@nokia.com> Reviewed-by: Alan Coopersmith <alan.coopersmith@oracle.com>
* Remove support for building without XCBJosh Triplett2010-06-031-1538/+2
| | | | | | | | | | | And there was much rejoicing. Signed-off-by: Josh Triplett <josh@joshtriplett.org> Reviewed-by: Jamey Sharp <jamey@minilop.net> Consensus on #xorg-devel agrees with removing --without-xcb; in particular, acks from Adam Jackson, Daniel Stone, Kristian Høgsberg, Julien Cristau, and Rémi Cardona.
* Merge branch 'xlib-xcb-thread-fixes'Jamey Sharp2010-05-101-10/+1
|\
| * Use InternalLockDisplay on code paths called from LockDisplay.Jamey Sharp2010-04-181-10/+1
| | | | | | | | | | | | | | | | It's easier to reason about the code when we can't re-enter the Xlib-private sync-handlers while they're already running. Signed-off-by: Jamey Sharp <jamey@minilop.net> Reviewed-by: Josh Triplett <josh@freedesktop.org>
* | Fix various build warningsJeremy Huddleston2010-04-231-0/+4
|/ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | imLcIm.c: In function '_XimCachedFileName': imLcIm.c:361: warning: format '%03x' expects type 'unsigned int', but argument 8 has type 'long unsigned int' imLcIm.c:364: warning: format '%03x' expects type 'unsigned int', but argument 8 has type 'long unsigned int' imRm.c: In function '_XimDefaultArea': imRm.c:597: warning: cast from pointer to integer of different size imRm.c: In function '_XimDefaultColormap': imRm.c:626: warning: cast from pointer to integer of different size lcFile.c:224: warning: no previous prototype for 'xlocaledir' lcUTF8.c: In function 'iconv_cstombs': lcUTF8.c:1841: warning: assignment discards qualifiers from pointer target type lcUTF8.c:1869: warning: pointer targets in passing argument 1 of 'wctomb' differ in signedness lcUTF8.c:1873: warning: pointer targets in passing argument 1 of 'wctomb' differ in signedness lcUTF8.c: In function 'iconv_mbstocs': lcUTF8.c:1935: warning: pointer targets in passing argument 2 of 'mbtowc' differ in signedness lcUTF8.c: In function 'iconv_mbtocs': lcUTF8.c:2031: warning: pointer targets in passing argument 2 of 'mbtowc' differ in signedness lcUTF8.c: In function 'iconv_mbstostr': lcUTF8.c:2121: warning: pointer targets in passing argument 2 of 'mbtowc' differ in signedness lcUTF8.c: In function 'iconv_strtombs': lcUTF8.c:2180: warning: pointer targets in passing argument 1 of 'wctomb' differ in signedness lcUTF8.c: In function '_XlcAddGB18030LocaleConverters': lcUTF8.c:2367: warning: passing argument 5 of '_XlcSetConverter' from incompatible pointer type lcUTF8.c:2368: warning: passing argument 5 of '_XlcSetConverter' from incompatible pointer type lcUTF8.c:2373: warning: passing argument 5 of '_XlcSetConverter' from incompatible pointer type lcUTF8.c:2374: warning: passing argument 5 of '_XlcSetConverter' from incompatible pointer type lcUTF8.c:2375: warning: passing argument 5 of '_XlcSetConverter' from incompatible pointer type lcUTF8.c:2376: warning: passing argument 5 of '_XlcSetConverter' from incompatible pointer type lcUTF8.c:2377: warning: passing argument 5 of '_XlcSetConverter' from incompatible pointer type XlibInt.c: In function '_XGetHostname': XlibInt.c:3441: warning: implicit declaration of function 'gethostname' XlibInt.c:3441: warning: nested extern declaration of 'gethostname' ConnDis.c: In function '_XDisconnectDisplay': ConnDis.c:540: warning: old-style function definition ConnDis.c: In function '_XSendClientPrefix': ConnDis.c:554: warning: old-style function definition ConnDis.c: In function 'XSetAuthorization': ConnDis.c:677: warning: old-style function definition Signed-off-by: Jeremy Huddleston <jeremyhu@apple.com>
* Stop returning an int from _XIDHandler and _XSeqSyncFunctionJosh Triplett2010-04-151-4/+2
| | | | | | | | | | _XIDHandler and _XSeqSyncFunction originally ran from dpy->synchandler, and thus had to return an int. Now, they only run from _XPrivSyncHandler or LockDisplay, neither of which needs to check their return value since they always returned 0. Make them both void. Signed-off-by: Josh Triplett <josh@freedesktop.org> Signed-off-by: Jamey Sharp <jamey@minilop.net>
* Move XID and sync handling from SyncHandle to LockDisplay to fix races.Jamey Sharp2010-04-151-12/+25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | XID and sync handling happened via _XPrivSyncHandler, assigned to dpy->synchandler and called from SyncHandle. _XPrivSyncHandler thus ran without the Display lock, so manipulating the Display caused races, and these races led to assertions in multithreaded code (demonstrated via ico). In the XTHREADS case, after you've called XInitThreads, we can hook LockDisplay and UnlockDisplay. Use that to run _XIDHandler and _XSeqSyncHandler from LockDisplay rather than SyncHandle; we then know that we hold the lock, and thus we can avoid races. We think it makes sense to do these both from LockDisplay rather than UnlockDisplay, so that you know you have valid sync and a valid XID before you start setting up the request you locked to prepare. In the !XTHREADS case, or if you haven't called XInitThreads, you don't get to use Xlib from multiple threads, so we can use the logic we have now (with synchandler and savedsynchandler) without any concern about races. This approach gets a bit exciting when the XID and sequence sync handlers drop and re-acquire the Display lock. Reacquisition will re-run the handlers, but they return immediately unless they have work to do, so they can't recurse more than once. In the worst case, if both of them have work to do, we can nest the Display lock three deep. In the case of the _XIDHandler, we drop the lock to call xcb_generate_id, which takes the socket back if it needs to request more XIDs, and taking the socket back will reacquire the lock; we take care to avoid letting _XIDHandler run again and re-enter XCB from the return_socket callback (which causes Very Bad Things, and is Not Allowed). Tested with ico (with 1 and 20 threads), and with several test programs for XID and sequence sync. Tested with and without XInitThreads(), and with and without XCB. Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=23192 Signed-off-by: Jamey Sharp <jamey@minilop.net> Signed-off-by: Josh Triplett <josh@freedesktop.org>
* Run the user's synchandler as well as any internal synchandlers.Jamey Sharp2010-04-121-0/+2
| | | | | | Fixes https://bugs.freedesktop.org/show_bug.cgi?id=27595 Signed-off-by: Jamey Sharp <jamey@minilop.net>
* Purge CVS/RCS id tagsAlan Coopersmith2010-01-141-2/+0
| | | | Signed-off-by: Alan Coopersmith <alan.coopersmith@sun.com>
* Fix compiler warning 'unused variable qelt'Peter Hutterer2009-07-291-1/+0
| | | | Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
* Add generic event cookie handling to libX11.Peter Hutterer2009-07-121-2/+182
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Generic events require more bytes than Xlib provides in the standard XEvent. Memory allocated by the extension and stored as pointers inside the event is prone to leak by simple 'while (1) { XNextEvent(...); }' loops. This patch adds cookie handling for generic events. Extensions may register a cookie handler in addition to the normal event vectors. If an extension has registered a cookie handler, _all_ generic events for this extensions must be handled through cookies. Otherwise, the default event handler is used. The cookie handler must return an XGenericEventCookie with a pointer to the data.The rest of the event (type, serialNumber, etc.) are to be filled as normal. When a client retrieves such a cookie event, the data is stored in an internal queue (the 'cookiejar'). This data is freed on the next call to XNextEvent(). New extension interfaces: XESetWireToEventCookie(display, extension_number, cookie_handler) Where cookie_handler must set cookie->data. The data pointer is of arbitray size and type but must be a single memory block. This memory block represents the actual extension's event. New client interfaces: XGetEventData(display, *cookie); XFreeEventData(display, *cookie); If the client needs the actual event data, it must call XGetEventData() with the cookie. This returns the data pointer (and removes it from the cookie jar) and the client is then responsible for freeing the event with XFreeEventData(). It is safe to call either function with a non-cookie event. Events unclaimed or not handled by the XGetEventData() are cleaned up automatically. Example client code: XEvent event; XGenericEventCookie *cookie = &ev; XNextEvent(display, &event); if (XGetEventData(display, cookie)) { XIEvent *xievent = cookie->data; ... } else if (cookie->type == GenericEvent) { /* handle generic event */ } else { /* handle extension/core event */ } XFreeEventData(display, cookie); Cookies are not multi-threading safe. Clients that use XGetEventData() must lock between XNextEvent and XGetEventData to avoid other threads freeing cookies. Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
* Drop ancient USG SysV #ifdefsAlan Coopersmith2009-06-161-1/+1
| | | | Signed-off-by: Alan Coopersmith <alan.coopersmith@sun.com>
* Fix a several sparse warnings: Using plain integer as NULL pointerAlan Coopersmith2009-04-061-1/+1
| | | | Signed-off-by: Alan Coopersmith <alan.coopersmith@sun.com>
* Bug 6820: Xlib shouldn't handle EAGAIN as a fatal IO errorAlan Coopersmith2009-03-121-7/+3
| | | | | | | X.Org Bug #6820 <http://bugs.freedesktop.org/show_bug.cgi?id=6820> Patch #17637 <http://bugs.freedesktop.org/attachment.cgi?id=17637> Signed-off-by: Alan Coopersmith <alan.coopersmith@sun.com>
* WORD64 compile fix. This bug catched on a overview of the code.Paulo Cesar Pereira de Andrade2009-02-021-1/+1
| | | | | | | | | | The code is wrong since the first git revision, so it seens that it has not been compiled with WORD64 for quite some time, there is also another interesting code in xkb/XKBRdBuf.c: <hash>ifdef WORD64 _XkbWriteCopyData32 Not Implemented Yet for sizeof(int)==8 <hash>endif and possibly there are other similar problems.
* Janitor: ansification, make distcheck, compiler warnings.Paulo Cesar Pereira de Andrade2009-01-281-7/+2
| | | | | | | | | | | Only convert to use "ansi prototypes" the functions warned from compilation with "./autogen.sh --prefix=/usr", on a Linux computer. Also, only address "trivial" compiler warning fixes in this commit. The new .gitignore is the output of a command like: % find . -name .gitignore -exec cat {} \; | sort | uniq and only the toplevel .gitignore file was kept.
* Support multiple independent internal sync handlersJamey Sharp2008-11-041-38/+47
| | | | | | | | | | | | | | | | | | | | Xlib has several independent tasks that need to be performed with the display unlocked. It does this by replacing the existing sync handler with one of a variety of internal sync handlers. However, if multiple internal sync handlers need to run, then the last one registering wins and previously registered internal sync handlers are never invoked. This manifested as a bug with DRI applications on Xlib/XCB as that requires both an XID handler after every XID allocation, and the periodic sequence number handler. The XID handler would win, and the sequence number handler would never be invoked. Fix this by unifying the internal sync handler mechanism into a single function that calls all of the known internal sync handlers. They all need to deal with being called when not strictly necessary now. Signed-off-by: Keith Packard <keithp@keithp.com> Signed-off-by: Jamey Sharp <jamey@minilop.net> Signed-off-by: Josh Triplett <josh@freedesktop.org>