xshm error handling improvement - now works with transparent fallback

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;