From dcd4aa8b3bd9d9bf3684fef2eaca524b1af1ff76 Mon Sep 17 00:00:00 2001 From: "Adam D. Ruppe" Date: Thu, 3 Jun 2021 11:46:57 -0400 Subject: [PATCH] xshm error handling improvement - now works with transparent fallback --- simpledisplay.d | 112 ++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 99 insertions(+), 13 deletions(-) diff --git a/simpledisplay.d b/simpledisplay.d index 2b4651e..84af046 100644 --- a/simpledisplay.d +++ b/simpledisplay.d @@ -5806,6 +5806,11 @@ version(X11) { return 0; } + private extern(C) int XShmErrorHandler (Display* dpy, XErrorEvent* evt) nothrow @nogc { + Image.impl.xshmfailed = true; + return 0; + } + private extern(C) int adrlogger (Display* dpy, XErrorEvent* evt) nothrow @nogc { import core.stdc.stdio; char[265] buffer; @@ -10988,9 +10993,10 @@ version(X11) { auto display = XDisplayConnection.get; auto font = XLoadQueryFont(display, xfontstr.ptr); // if the user font choice fails, fixed is pretty reliable (required by X to start!) and not bad either - xfontstr = "-*-fixed-medium-r-*-*-13-*-*-*-*-*-*-*"; - if(font is null) + if(font is null) { + xfontstr = "-*-fixed-medium-r-*-*-13-*-*-*-*-*-*-*"; font = XLoadQueryFont(display, xfontstr.ptr); + } char** lol; int lol2; @@ -12267,10 +12273,21 @@ mixin DynamicLoad!(XRender, "Xrender", 1, false, true) XRenderLibrary; private __gshared char* displayName; private __gshared int connectionSequence_; + private __gshared bool isLocal_; /// use this for lazy caching when reconnection static int connectionSequenceNumber() { return connectionSequence_; } + /++ + Guesses if the connection appears to be local. + + History: + Added June 3, 2021 + +/ + static @property bool isLocal() nothrow @trusted @nogc { + return isLocal_; + } + /// Attempts recreation of state, may require application assistance /// You MUST call this OUTSIDE the event loop. Let the exception kill the loop, /// then call this, and if successful, reenter the loop. @@ -12434,6 +12451,17 @@ mixin DynamicLoad!(XRender, "Xrender", 1, false, true) XRenderLibrary; if(!librariesSuccessfullyLoaded) throw new Exception("Unable to load X11 client libraries"); display = XOpenDisplay(displayName); + + + auto str = display.display_name; + // this is a bit of a hack but like if it looks like a unix socket we assume it is local + // and otherwise it probably isn't + if(str is null || (str[0] != ':' && str[0] != '/')) + isLocal_ = false; + else + isLocal_ = true; + + //XSetErrorHandler(&adrlogger); //XSynchronize(display, true); connectionSequence_++; @@ -12511,11 +12539,7 @@ mixin DynamicLoad!(XRender, "Xrender", 1, false, true) XRenderLibrary; int i1, i2, i3; xshmQueryCompleted = true; - auto str = XDisplayConnection.get().display_name; - // only if we are actually on the same machine does this - // have any hope, and the query extension only asks if - // the server can in theory, not in practice. - if(str is null || (str[0] != ':' && str[0] != '/')) + if(!XDisplayConnection.isLocal) _xshmAvailable = false; else _xshmAvailable = XQueryExtension(XDisplayConnection.get(), "MIT-SHM", &i1, &i2, &i3) != 0; @@ -12526,6 +12550,8 @@ mixin DynamicLoad!(XRender, "Xrender", 1, false, true) XRenderLibrary; bool usingXshm; final: + private __gshared bool xshmfailed; + void createImage(int width, int height, bool forcexshm=false, bool enableAlpha = false) { auto display = XDisplayConnection.get(); assert(display !is null); @@ -12534,6 +12560,22 @@ mixin DynamicLoad!(XRender, "Xrender", 1, false, true) XRenderLibrary; // it will only use shared memory for somewhat largish images, // since otherwise we risk wasting shared memory handles on a lot of little ones if (xshmAvailable && (forcexshm || (width > 100 && height > 100))) { + + + // it is possible for the query extension to return true, the DISPLAY check to pass, yet + // the actual use still fails. For example, if the program is in a container and permission denied + // on shared memory, or if it is a local thing forwarded to a remote server, etc. + // + // If it does fail, we need to detect it now, abort the xshm and fall back to core protocol. + + + // synchronize so preexisting buffers are clear + XSync(display, false); + xshmfailed = false; + + auto oldErrorHandler = XSetErrorHandler(&XShmErrorHandler); + + usingXshm = true; handle = XShmCreateImage( display, @@ -12543,22 +12585,66 @@ mixin DynamicLoad!(XRender, "Xrender", 1, false, true) XRenderLibrary; null, &shminfo, width, height); - assert(handle !is null); + if(handle is null) + goto abortXshm1; + + if(handle.bytes_per_line != 4 * width) + goto abortXshm2; - assert(handle.bytes_per_line == 4 * width); shminfo.shmid = shmget(IPC_PRIVATE, handle.bytes_per_line * height, IPC_CREAT | 511 /* 0777 */); - //import std.conv; import core.stdc.errno; - assert(shminfo.shmid >= 0);//, to!string(errno)); + if(shminfo.shmid < 0) + goto abortXshm3; handle.data = shminfo.shmaddr = rawData = cast(ubyte*) shmat(shminfo.shmid, null, 0); - assert(rawData != cast(ubyte*) -1); + if(rawData == cast(ubyte*) -1) + goto abortXshm4; shminfo.readOnly = 0; XShmAttach(display, &shminfo); + + // and now to the final error check to ensure it actually worked. + XSync(display, false); + if(xshmfailed) + goto abortXshm5; + + XSetErrorHandler(oldErrorHandler); + XDisplayConnection.registerImage(this); // if I don't flush here there's a chance the dtor will run before the // ctor and lead to a bad value X error. While this hurts the efficiency // it is local anyway so prolly better to keep it simple XFlush(display); + + return; + + abortXshm5: + shmdt(shminfo.shmaddr); + rawData = null; + + abortXshm4: + shmctl(shminfo.shmid, IPC_RMID, null); + + abortXshm3: + // nothing needed, the shmget failed so there's nothing to free + + abortXshm2: + XDestroyImage(handle); + handle = null; + + abortXshm1: + XSetErrorHandler(oldErrorHandler); + usingXshm = false; + handle = null; + + shminfo = typeof(shminfo).init; + + _xshmAvailable = false; // don't try again in the future + + //import std.stdio; writeln("fallingback"); + + goto fallback; + } else { + fallback: + if (forcexshm) throw new Exception("can't create XShm Image"); // This actually needs to be malloc to avoid a double free error when XDestroyImage is called import core.stdc.stdlib : malloc; @@ -13182,7 +13268,7 @@ mixin DynamicLoad!(XRender, "Xrender", 1, false, true) XRenderLibrary; ; // xshm is our shortcut for local connections - if(Image.impl.xshmAvailable || forceIncludeMouseMotion) + if(XDisplayConnection.isLocal || forceIncludeMouseMotion) mask |= EventMask.PointerMotionMask; else mask |= EventMask.ButtonMotionMask;