| Summary: | Fatal error message when we close libX11 window application. | ||
|---|---|---|---|
| Product: | XCB | Reporter: | Arvind Umrao <arvind.umrao> |
| Component: | Library | Assignee: | Arvind Umrao <arvind.umrao> |
| Status: | RESOLVED FIXED | QA Contact: | xcb mailing list dummy <xcb> |
| Severity: | normal | ||
| Priority: | medium | Keywords: | patch |
| Version: | unspecified | ||
| Hardware: | Other | ||
| OS: | Solaris | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
| Attachments: |
Patch fix
Patch fix Patch fix |
||
Created attachment 51992 [details] [review] Patch fix XCB should give same error messages as legacy libX11 were. In order to fix this issue, I have set right error number and made code similar to lib11:XlibInt.c:_XRead: ESET(EPIPE). I believe xcb_in.c is the best place to fix this error,rather than libX11:xcb_io.c. KCB was returning errno EAGAIN, I made it return EPIPE to make it similar to legacy libX11. Review of attachment 51992 [details] [review]: You shouldn't set the EPIPE error. The _xcb_conn_shutdown function already signals the connection errors. So instead of checking for EPIPE, you could introduce a version of _xcb_conn_shutdown that would make it possible to give a specific value for xcb_connection_t::has_error. This value could then be used to display different error messages in IO error handler. (In reply to comment #2) > Review of attachment 51992 [details] [review]: > > You shouldn't set the EPIPE error. The _xcb_conn_shutdown function already > signals the connection errors. So instead of checking for EPIPE, you could > introduce a version of _xcb_conn_shutdown that would make it possible to give a > specific value for xcb_connection_t::has_error. This value could then be used > to display different error messages in IO error handler. good review. Thanks But right now XCB gives message number EAGAIN. I do not think EAGAIN is very appropriate error message when connection is already broken. If you think it is correct error message, then I have another fix, just change default io error hander of linX11, so that it does not generate fatal error for error case EAGAIN. Right now in XCB, xcb_connection_t::has_error is harcoded to one. As you said xcb_connection_t::has_error is not very useful for IO handler and I do agree with you. I will be happy if somehow libX11 error hander does not print fatal error message, but print some sensible message. So we have two solutions, let me know which is best one 1) PATCH I have given, make errno= EPIPE , because legacy libX11 is also doing the same. 2) If you think XCB is giving correct error number, then just change default io error hander of linX11, so that it does not generate fatal error for error case EAGAIN. (In reply to comment #3) > But right now XCB gives message number EAGAIN. I do not think EAGAIN is very > appropriate error message when connection is already broken. If you think it is > correct error message, then I have another fix, just change default io error > hander of linX11, so that it does not generate fatal error for error case > EAGAIN. When connection is closed by X server, read/recv returns zero and doesn't set any error numbers. The EAGAIN that you see must therefore come from an earlier system call. > So we have two solutions, let me know which is best one > 1) PATCH I have given, make errno= EPIPE , because legacy libX11 is also doing > the same. > > 2) If you think XCB is giving correct error number, then just change default io > error hander of linX11, so that it does not generate fatal error for error case > EAGAIN. Neither of these solutions is acceptable. You have no way of knowing which system call has set errno when IO error handler is called. Just set a proper error code in xcb_connection_t and use that (reuse has_error or add a new field). (In reply to comment #4) > (In reply to comment #3) > > But right now XCB gives message number EAGAIN. I do not think EAGAIN is very > > appropriate error message when connection is already broken. If you think it is > > correct error message, then I have another fix, just change default io error > > hander of linX11, so that it does not generate fatal error for error case > > EAGAIN. > > When connection is closed by X server, read/recv returns zero and doesn't set > any error numbers. The EAGAIN that you see must therefore come from an earlier > system call. > Yes you are right. EAGAIN is comming from pervious system calls. Thats why erron values are wrong. Our legacy libX11 hardcoded it with EPIPE. > > So we have two solutions, let me know which is best one > > 1) PATCH I have given, make errno= EPIPE , because legacy libX11 is also doing > > the same. > > > > 2) If you think XCB is giving correct error number, then just change default io > > error hander of linX11, so that it does not generate fatal error for error case > > EAGAIN. > > Neither of these solutions is acceptable. You have no way of knowing which > system call has set errno when IO error handler is called. Just set a proper > error code in xcb_connection_t and use that (reuse has_error or add a new > field). I do agree with you but I do not want to make big changes. libX11 is wrapper over xcb. libX11 default io hander print error message after filtering error variable erron. I am afraid changing any thing at libX11:XlibInt.c:_XDefaultIOError() is risky. So I have to set correct value of erron some where. If you do not want me to set value of erron at xcb, can I set it at libX11:xcb_io.c.? Like this way if(xcb_connection_has_error(dpy->xcb->connection)) { erron=EPIPE; //or ESET(EPIPE) Even our legacy libX11 is also hardcoing it in same way _XIOError(dpy); } Let me know if I should send you another patch with above changes. Created attachment 52408 [details]
Patch fix
Created attachment 52507 [details] [review] Patch fix Patch merged. |
Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.
When we close any libX11 window application, we get following fatal error message. Fatal error message comes only with libX11 which are based on xcb. XIO: fatal IO error 11 (Resource temporarily unavailable) on X server ":0.0" after 14 requests (13 known processed) with 0 events remaining. Error is simply because I quit the application, without exiting while loop of XNextEvent. Fatal error will not occur if we process Window Close Event of Window-Manager through event handler so XNextEvent does not fail, with adding following line Atom delWindow = XInternAtom( d, "WM_DELETE_WINDOW", 0 ); XSetWMProtocols(d , w, &delWindow, 1); while(1) { XNextEvent(d, &e); ... if(e.type==ClientMessage) { printf("Client Messages\n"); break; } .... } XCB should give same error message as our legacy xlib was giving. Fatal error message is very confusing to developers, when they run simple 20 lines of following test sample. Step to reproduce the problem Compile following code with cc test.c -lX11 then run ./a.out close the application by clicking X icon #include <X11/Xlib.h> #include <stdio.h> #include <stdlib.h> #include <string.h> int main(void) { Display *d; Window w; XEvent e; char *msg = "Hello, World!"; int s; /* open connection with the server */ d = XOpenDisplay(NULL); if (d == NULL) { fprintf(stderr, "Cannot open display\n"); exit(1); } s = DefaultScreen(d); /* create window */ w = XCreateSimpleWindow(d, RootWindow(d, s), 10, 10, 200, 200, 1, BlackPixel(d, s), WhitePixel(d, s)); /* select kind of events we are interested in */ XSelectInput(d, w, ExposureMask | KeyPressMask); /* map (show) the window */ XMapWindow(d, w); /* event loop */ while (1) { XNextEvent(d, &e); /* draw or redraw the window */ if (e.type == Expose) { XFillRectangle(d, w, DefaultGC(d, s), 20, 20, 10, 10); XDrawString(d, w, DefaultGC(d, s), 50, 50, msg, strlen(msg)); } /* exit on key press */ if (e.type == KeyPress) break; } /* close connection to server */ XCloseDisplay(d); return 0; }