Re: Fixes to X11_waitforinput()

[Ethan M. <eam...@gm...>]

On Saturday, 14 March 2020 23:12:24 PDT Dima Kogan wrote:
> Hi.
> 
> I've been working on fixing broken "pause mouse close" behavior on the
> x11 terminal while x-forwarding. This is
> 
>   https://sourceforge.net/p/gnuplot/bugs/2208/
> 
> In the process I discovered that I already encountered weirdness in this
> area previously:
> 
>   https://sourceforge.net/p/gnuplot/mailman/message/36440354/
> 
> I just fixed all the issues, I think. These are in 2 patches on this
> branch:
> 
>   https://github.com/dkogan/gnuplot/commits/fixed-x11-waitforinput

Can you send me an idiot's guide to how to extract those patches from
the github site?   I can view them as html pages, but cannot figure out
how to download in a form I can feed to `patch`. 

Alternatively can you tell me, still a naive git user, how to fetch or
cherry-pick these commits from your git instance?

	thanks,

		Ethan


> The first patch improves diagnostic printing in this area. Instead of
> printing enums as NUMBERS, it prints the enum NAMES. This is infinitely
> more useful.
> 
> The second patch contains the actual fixes. It looks more invasive than
> it is. 99% of the modifications are in X11_waitforinput(). And much of
> it is indentation.
> 
> There were several issues, all of them caused by hoaky input
> multiplexing. I'm attaching two very simple gnuplot scripts that tickled
> the bug. To observe the problems you need to slow down the X connection
> by ssh-ing to any server, and trying to plot through x-forwarding.
> Before any of the fixes this would happen:
> 
>     $ gnuplot < mild.pause.mouse.close.broken.gp
> 
>     gnuplot> ause mouse close
>              ^
>              line 0: invalid command
> 
> This was caused by this block of code:
> 
>     if (IPC_LOCK) {
> #ifdef HAVE_USLEEP
> 	usleep(100);
> #endif
>         if (repeat_count++ < 10000)
> 	    goto AGAIN;
>     }
> 
> The slower link means that 10000 is too low. Bumping it up fixes the
> problem for this data file. But if you then run the other test file, you
> still see issues:
> 
>     $ gnuplot < hard.pause.mouse.close.broken.gp
> 
>     gnuplot> splse mouse close
>              ^
>              line 0: invalid command
> 
> This is an entirely different bug. I chased it down a few days ago, so I
> don't remember the exact details. The gist of the issue was that we
> would read some commands from ipc_back_fd then we would read some stuff
> from stdin, and then we would go back, and read the rest of the stuff
> from ipc_back_fd. This non-atomicity would break stuff. The fix was to
> read ALL the available ipc_back_fd data when possible, BEFORE reading
> anything from stdin.
> 
> The existing code was hoaky and mysterious. There were weird sleeps all
> over the place, and lots of state. The patch here refactors
> X11_waitforinput() to get rid of the sleeps and unneeded loops. The
> IPC_LOCK is gone, and is now another possible option argument to
> X11_waitforinput(). And there're better comments, I think.
> 
> It works for me so far, but this is an error-prone area. Can somebody
> take a look? The new approach is much better, I think, but more testing
> would be good.
> 
> Thanks.
> 
>