Improve headlessness detection for backend selection.

anntzer · anntzer · commit 653f3d04e60e · 2020-08-22T11:52:31.000+02:00
We currently check the $DISPLAY environment variable to autodetect
whether we should auto-pick a non-interactive backend on Linux, but that
variable can be set to an "invalid" value.  A realistic use case is for
example a tmux session started interactively inheriting an initially
valid $DISPLAY, but to which one later reconnects e.g. via ssh, at which
point $DISPLAY becomes invalid.

Before this PR, something like
```
DISPLAY=:123 MPLBACKEND= MATPLOTLIBRC=/dev/null python -c 'import pylab'
```
(where we unset matplotlibrc to force backend autoselection) would crash
when we select qt and qt fails to initialize as $DISPLAY is invalid (qt
unconditionally abort()s via qFatal() in that case).
With this PR, we correctly autoselect a non-interactive backend.

Note that we were *already* relying on $DISPLAY being correctly set
before, and this PR also doesn't return "invalid display" if we can't
load X11, so this should not make anything worse on Wayland (at worst
we'll just fail to detect headlessness like before).
diff --git a/lib/matplotlib/backend_bases.py b/lib/matplotlib/backend_bases.py
@@ -2755,10 +2755,15 @@ def show(self):
         warning in `.Figure.show`.
         """
         # This should be overridden in GUI backends.
-        if cbook._get_running_interactive_framework() != "headless":
-            raise NonGuiException(
-                f"Matplotlib is currently using {get_backend()}, which is "
-                f"a non-GUI backend, so cannot show the figure.")
+        if sys.platform == "linux" and not os.environ.get("DISPLAY"):
+            # We cannot check _get_running_interactive_framework() ==
+            # "headless" because that would also suppress the warning when
+            # $DISPLAY exists but is invalid, which is more likely an error and
+            # thus warrants a warning.
+            return
+        raise NonGuiException(
+            f"Matplotlib is currently using {get_backend()}, which is a "
+            f"non-GUI backend, so cannot show the figure.")
 
     def destroy(self):
         pass
diff --git a/lib/matplotlib/backends/backend_qt5.py b/lib/matplotlib/backends/backend_qt5.py
@@ -7,7 +7,6 @@
 import traceback
 
 import matplotlib
-
 from matplotlib import backend_tools, cbook
 from matplotlib._pylab_helpers import Gcf
 from matplotlib.backend_bases import (
@@ -113,10 +112,8 @@ def _create_qApp():
                     is_x11_build = False
             else:
                 is_x11_build = hasattr(QtGui, "QX11Info")
-            if is_x11_build:
-                display = os.environ.get('DISPLAY')
-                if display is None or not re.search(r':\d', display):
-                    raise RuntimeError('Invalid DISPLAY variable')
+            if is_x11_build and matplotlib._c_internal_utils.invalid_display():
+                raise RuntimeError('Invalid DISPLAY variable')
 
             try:
                 QtWidgets.QApplication.setAttribute(
diff --git a/lib/matplotlib/cbook/__init__.py b/lib/matplotlib/cbook/__init__.py
@@ -73,7 +73,8 @@ def _get_running_interactive_framework():
     if 'matplotlib.backends._macosx' in sys.modules:
         if sys.modules["matplotlib.backends._macosx"].event_loop_is_running():
             return "macosx"
-    if sys.platform.startswith("linux") and not os.environ.get("DISPLAY"):
+    if (sys.platform.startswith("linux")
+            and _c_internal_utils.invalid_display()):
         return "headless"
     return None
 
diff --git a/setupext.py b/setupext.py
@@ -349,8 +349,10 @@ def get_extensions(self):
         # c_internal_utils
         ext = Extension(
             "matplotlib._c_internal_utils", ["src/_c_internal_utils.c"],
-            libraries=({"win32": ["ole32", "shell32", "user32"]}
-                       .get(sys.platform, [])))
+            libraries=({
+                "linux": ["dl"],
+                "win32": ["ole32", "shell32", "user32"],
+            }.get(sys.platform, [])))
         yield ext
         # contour
         ext = Extension(
diff --git a/src/_c_internal_utils.c b/src/_c_internal_utils.c
@@ -1,11 +1,44 @@
 #define PY_SSIZE_T_CLEAN
 #include <Python.h>
+#ifdef __linux__
+#include <dlfcn.h>
+#endif
 #ifdef _WIN32
 #include <Objbase.h>
 #include <Shobjidl.h>
 #include <Windows.h>
 #endif
 
+static PyObject* mpl_invalid_display(PyObject* module)
+{
+#ifdef __linux__
+    void* libX11;
+    typedef struct Display Display;
+    Display* (* XOpenDisplay)(char* display_name);
+    int (* XCloseDisplay)(Display* display);
+    if (!(libX11 = dlopen("libX11.so", RTLD_LAZY)) ||
+        !(XOpenDisplay = dlsym(libX11, "XOpenDisplay")) ||
+        !(XCloseDisplay = dlsym(libX11, "XCloseDisplay"))) {
+        Py_RETURN_FALSE;
+    }
+    Display* display = XOpenDisplay(NULL);
+    if (display) {
+        XCloseDisplay(display);
+    }
+    if (dlclose(libX11)) {
+        PyErr_SetString(PyExc_RuntimeError, dlerror());
+        return NULL;
+    }
+    if (display) {
+        Py_RETURN_FALSE;
+    } else {
+        Py_RETURN_TRUE;
+    }
+#else
+    Py_RETURN_FALSE;
+#endif
+}
+
 static PyObject* mpl_GetCurrentProcessExplicitAppUserModelID(PyObject* module)
 {
 #ifdef _WIN32
@@ -66,6 +99,11 @@ static PyObject* mpl_SetForegroundWindow(PyObject* module, PyObject *arg)
 }
 
 static PyMethodDef functions[] = {
+    {"invalid_display", (PyCFunction)mpl_invalid_display, METH_NOARGS,
+     "invalid_display()\n--\n\n"
+     "Attempt to check whether the current X11 display is invalid.\n\n"
+     "Returns True if running on Linux and libX11 can be loaded and \n"
+     "XOpenDisplay(NULL) returns NULL, False otherwise."},
     {"Win32_GetCurrentProcessExplicitAppUserModelID",
      (PyCFunction)mpl_GetCurrentProcessExplicitAppUserModelID, METH_NOARGS,
      "Win32_GetCurrentProcessExplicitAppUserModelID()\n--\n\n"
