Path: utzoo!attcan!cmtl01!matrox!uvm-gen!uunet!lll-winken!ames!pasteur!ucbvax!bloom-beacon!szilard.cray.COM!scott From: scott@szilard.cray.COM (Scott Bolte) Newsgroups: comp.windows.x Subject: bug fix for XtAddInput() Message-ID: <8901260143.AA03375@hal.cray.com> Date: 26 Jan 89 01:43:41 GMT Sender: daemon@bloom-beacon.MIT.EDU Organization: The Internet Lines: 248 I have seen what appears to be a bug in X11R3 that occurs when using XtAddInput(). The routine that I pass in to be used to read input is called when there is no data. This results in a blocking read. This problem was reported officially a few weeks ago by our local X-pert. There has been no response yet so I tracked down the problem myself this today. A description, fix, and test case follow... The problem lies in the logic of XtAppNextEvent(). Due to the way in which it calls _XtwaitForSomething() and DoOtherSources() it is possible for two duplicate InputEvent entries to be added to the outstandingQueue. A blow by blow description is included as a comment in the workaround that follows. First a disclaimer: The code that follows fixes the symptom of the error instead of the actual problem. If I am correct a proper fix should (eventually :-) come from the X consortium. Given their workload it might be some time. In the meantime, if you too are having problems, apply the fix. Due to the informal nature of this posting I suggest you look at my logic before you make use of the fix. (After all, this is the first time that I looked at the NextEvent code.) What I do is scan the entire outstandingQueue before I add an InputEvent. If there is already an entry on the queue for the same source I do not do the insertion. Admittedly this will slow down processing the most at the worst time, when there are a lot of events queued. It is the most non-intrusive solution that I could come up with though. If anyone sees either a problem with this scheme or a better solution please post it as soon as possible. Scott Bolte Cray Research, Inc. scott@cray.com The "diff -c" listing of the old and new lib/Xt/NextEvent.c files..... *** /usr/galileo1/X11R3/lib/Xt/NextEvent.c Fri Oct 21 12:23:48 1988 --- NextEvent.c Wed Jan 25 17:35:39 1989 *************** *** 214,236 **** } } ! app->selectRqueue[i]->ie_oq = app->outstandingQueue; ! app->outstandingQueue = app->selectRqueue[i]; nfound--; } if (FD_ISSET (i, &wmaskfd)) { ! app->selectWqueue[i]->ie_oq = app->outstandingQueue; ! app->outstandingQueue = app->selectWqueue[i]; nfound--; } if (FD_ISSET (i, &emaskfd)) { ! app->selectEqueue[i]->ie_oq = app->outstandingQueue; ! app->outstandingQueue = app->selectEqueue[i]; nfound--; } ENDILOOP: ; } return ret; } static void IeCallProc(ptr) --- 214,291 ---- } } ! if( UniqueEntry(app->outstandingQueue,app->selectRqueue[i])){ ! app->selectRqueue[i]->ie_oq = app->outstandingQueue; ! app->outstandingQueue = app->selectRqueue[i]; ! } nfound--; } if (FD_ISSET (i, &wmaskfd)) { ! if( UniqueEntry(app->outstandingQueue,app->selectWqueue[i])){ ! app->selectWqueue[i]->ie_oq = app->outstandingQueue; ! app->outstandingQueue = app->selectWqueue[i]; ! } nfound--; } if (FD_ISSET (i, &emaskfd)) { ! if( UniqueEntry(app->outstandingQueue,app->selectEqueue[i])){ ! app->selectEqueue[i]->ie_oq = app->outstandingQueue; ! app->outstandingQueue = app->selectEqueue[i]; ! } nfound--; } ENDILOOP: ; } return ret; + } + + /* + * Insure that there are not any entries in the "ie_queue" that match + * the "ie_ptr". If there are return 0 otherwise return 1. + * + * This routine is a workaround that fixes a symptom rather than + * the incorrect logic of the input handler. It prevents more than + * one entry for any single source. + * + * The problem is in XtAppNextEvent(). What would happen is... + * + * 1) XtAppNextEvent() calls _XtwaitForSomething() directly. + * + * 2) data would come in on file descriptor FRED while + * _XtwaitForSomething() is blocked in select. + * + * 3) _XtwaitForSomething() will add the corresponding + * app->selectRqueue[FRED] to the outstandingQueue. + * + * 4) control returns back up to XtAppNextEvent(). + * + * 5) DoOtherSources() is called which in turns calls _XtwaitForSomething(). + * + * 6) _XtwaitForSomething() sees there is still data on FRED and adds + * a second selectRqueue[FRED] entry. + * + * 7) DoOtherSources() now calls IeCallProc() which results in the data + * being read off FRED. + * + * 8) one InputEvent is removed from the outstandingQueue. + * + * 9) DoOtherSources() sees the second entry on the outstandingQueue and + * calls IeCallProc() again. Unfortunately there no longer is any data to + * be read from FRED. + * + * 10) Twiddle(left_thumb,right_thumb) 'til data comes in. + * + */ + static int UniqueEntry(ie_ptr,ie_queue) + InputEvent *ie_ptr,*ie_queue; + { + register InputEvent *ptr; + + for( ptr = ie_queue ; ptr != (InputEvent *) 0 ; ptr = ptr->ie_oq ){ + if( ptr == ie_ptr ) + return(0); + } + return(1); } static void IeCallProc(ptr) ========================================================================= = = = End of diff, start of test case. = = = ========================================================================= #include #include #include #include #include main(argc, argv) int argc; char *argv[]; { int pipes[2], read_fd(); void time_out(); pipe(pipes); fprintf(stderr, "read pipe is %d and write pipe is %d\n", pipes[0], pipes[1]); (void) XtInitialize("master", "crayperf", NULL, 0, &argc, argv); (void) XtAddTimeOut(5000, time_out, pipes[1]); (void) XtAddInput(pipes[0], XtInputReadMask, read_fd, NULL); XtMainLoop(); } void time_out(fd, id) int fd; XtIntervalId *id; { int i; fprintf(stderr, "time_out() was called with fd %d.\n", fd); i = write(fd, "X", 1); fprintf(stderr, "time_out() wrote %d %s to fd %d\n", i ,i > 1 ? "bytes" : "byte", fd); XtAddTimeOut(5000, time_out, fd); } read_fd(junk, socket, InId) caddr_t junk; /* Client data for X callbacks */ int *socket; caddr_t InId; /* XtInputId if we were to use it */ { long t; char c; int i; t = time(0); fprintf(stderr, "read_fd() called at %ld.\n", t); if (read_check(*socket) <= 0) { fprintf(stderr, " ****** I do not agree that there is data.\n"); return; } i = read(*socket, &c, 1); switch (i) { case -1: perror("read returned an error. "); break; case 0: fprintf(stderr, "read returned zero bytes.\n"); break; case 1: fprintf(stderr, "read returned the '%c' character.\n", c); break; default: fprintf(stderr, "read returned more than a single character!\n"); break; } } read_check(fd) int fd; { int status; int rmask; struct timeval tv; tv.tv_sec = 0; tv.tv_usec = 0; rmask = (1 << fd); status = select(fd + 1, &rmask, 0, 0, &tv); return (status); }