FDH2 / UxPlay

AirPlay Unix mirroring server
GNU General Public License v3.0
1.34k stars 72 forks source link

Raspberry Pi crashes on connection, cause and possible solution #212

Closed march1993 closed 9 months ago

march1993 commented 9 months ago

I got BadWindow crash every single time I started mirroring my screen. After digging in the source code, I found out that enum_windows would crash the process. The problem would be gone if I add sleep(1); before calling enum_windows function. The cause might be that the window became invalid soon after calling XQueryTree. The solution is straightforward, call XSetErrorHandler to a dummy handler to ignore all the errors when enumerating the windows.

fduncanh commented 9 months ago

Thanks for this comment. Can you give more info about the Pi (model = ?) and the OS (Raspberry Pi OS bullseye?)

void get_x_window(X11_Window_t * X11, const char * name) {
    Window root = XDefaultRootWindow(X11->display);     

    sleep(1);    <================================== is this the proposed change?  (1 sec is a long time...)
                         (or should  should XSetErrorHandler be used somwhere)

    X11->window  = enum_windows(name, X11->display, root, 0);
#ifdef ZOOM_WINDOW_NAME_FIX
    if (X11->window) {
        Atom _NET_WM_NAME = XInternAtom(X11->display, "_NET_WM_NAME", 0);
        Atom UTF8_STRING = XInternAtom(X11->display, "UTF8_STRING", 0);
        XChangeProperty(X11->display, X11->window, _NET_WM_NAME, UTF8_STRING, 
                        8, 0, (const unsigned char *) name, strlen(name));
        XSync(X11->display, False);
    }
#endif
}
fduncanh commented 9 months ago

The rest of the code, for discussion.

Please suggest the details of the XSetErrorHandler fix.

Window enum_windows(const char * str, Display * display, Window window, int depth) {
    int i;
    XTextProperty text;
    XGetWMName(display, window, &text);
    char* name;
    XFetchName(display, window, &name);
    if (name != 0 &&  strcmp(str, name) == 0) {
        return window;
    }
    Window _root, parent;
    Window* children;
    unsigned int n;
    XQueryTree(display, window, &_root, &parent, &children, &n);
    if (children != NULL) {
        for (i = 0; i < n; i++) {
            Window w = enum_windows(str, display, children[i], depth + 1);
            if (w) return w;
        }
        XFree(children);
    }
    return (Window) NULL;
}
march1993 commented 9 months ago

Thanks for this comment. Can you give more info about the Pi (model = ?) and the OS (Raspberry Pi OS bullseye?)

void get_x_window(X11_Window_t * X11, const char * name) {
    Window root = XDefaultRootWindow(X11->display);     

    sleep(1);    <================================== is this the proposed change?  (1 sec is a long time...)
                         (or should  should XSetErrorHandler be used somwhere)

    X11->window  = enum_windows(name, X11->display, root, 0);
#ifdef ZOOM_WINDOW_NAME_FIX
    if (X11->window) {
        Atom _NET_WM_NAME = XInternAtom(X11->display, "_NET_WM_NAME", 0);
        Atom UTF8_STRING = XInternAtom(X11->display, "UTF8_STRING", 0);
        XChangeProperty(X11->display, X11->window, _NET_WM_NAME, UTF8_STRING, 
                        8, 0, (const unsigned char *) name, strlen(name));
        XSync(X11->display, False);
    }
#endif
}

My device is CM4 and the OS is latest bullseye. Yes, I added sleep(1); at the same position. And you are right, 1 sec is a super long time. So I suggest call XSetErrorHandler. The code would be like,

int catcher( Display *disp, XErrorEvent *xe ) {
        // do anything
        return 0;
}

XSetErrorHandler(catcher);
X11->window  = enum_windows(name, X11->display, root, 0);
XSetErrorHandler(NULL);

Window enum_windows(const char * str, Display * display, Window window, int depth) {
    int i;
    XTextProperty text;
    XGetWMName(display, window, &text);
    char* name = NULL; // <- I'm not sure whether the name pointer would be set empty by XFetchName when it fails, so in case x11 lib doesn't touch it, I suggest initialize the variable to NULL explicitly。
    XFetchName(display, window, &name);
    if (name != 0 &&  strcmp(str, name) == 0) {
        return window;
    }
    Window _root, parent;
    Window* children = NULL; // <- same as XFetchName
    unsigned int n;
    XQueryTree(display, window, &_root, &parent, &children, &n);
    if (children != NULL) {
        for (i = 0; i < n; i++) {
            Window w = enum_windows(str, display, children[i], depth + 1);
            if (w) return w;
        }
        XFree(children);
    }
    return (Window) NULL;
}

I haven't tested the approach yet. I will let you know whether it works ASAP.

fduncanh commented 9 months ago

X11 is a bit of an arcane mystery....

https://www.remlab.net/op/xlib.shtml

I understand what its doing: your code seems clean and doesn't seem to have any bad effects in my tests, so let's see if it fixes your issue! (which I haven't see happening on a regular R Pi 4B).