On Fri, Jun 05, 2020 at 03:05:49PM -0400, Kyryl Melekhin wrote:
> This bug is very exceptional and will only affect users who run other
> terminal programs with st.
> For example running
> " st -e vim . "
> " st -e lynx "
> etc ...
>
Hi Kyryl,
Thanks for the report.
> The bug happens randomly, and on some systems may not even exist, very
> hard to reproduce and predict. On my system with clean st build
> changes are 50/50. Race condition depends heavily on timing. The
> problem lies in usage of fork() function inside ttynew() function.
> Basically what will happen is that the child program has a chance of
> getting wrong window dimensions assigned, and if that program is vim
> or lynx it will permanently mess up every single character in the
> program and rendering, If you can't reproduce the bug right now, just
> put usleep between
>
> (x.c 1890):
> ttyfd = ttynew(opt_line, shell, opt_io, opt_cmd);
>
> /*
> usleep doc: usec is greater than or equal to 1000000. (On systems where
> that is considered an error.)
> */
> /* Add this; value must not error, print Success */
> fprintf(stderr, "%s\n", strerror(usleep(999999)));
> cresize(w, h);
>
This part is incorrect: "The usleep() function returns the 0 if successful;
otherwise the -1 is returned and the global variable errno is set to indicate
the error. So it does not reset errno if it was succesful (happened on my
system).
So this would be better/good enough for debug printing:
errno = 0;
ret = usleep(999999);
fprintf(stderr, "usleep, ret=%d, errno=%d, err=%s\n", ret, errno, strerror(errno));
> that should make it 100% consistent. Also use monocle layout in dwm to
> see better. I think this bug happens only if the child process starts
> to execute its own window dimensions code before st's calls cresize(w,
> h).
> That is why the sleep function makes the bug consistent.
> I propose a solution to this that worked for me:
>
> diff --git a/x.c b/x.c
> index 210f184..fb80366 100644
> --- a/x.c
> +++ b/x.c
> _AT_@ -1887,6 +1887,7 @@ run(void)
> }
> } while (ev.type != MapNotify);
>
> + cresize(w, h);
> ttyfd = ttynew(opt_line, shell, opt_io, opt_cmd);
> cresize(w, h);
>
> Calling cresize before fork() ensures that the child starts with
> correct dimensions already set (since it inherits).
> For me this has reduced the chance of bug to 0%. However for some
> reasons I may not know if you still keep that
> usleep call there, things yet again go south. But on the other side,
> if this fix helped me to never ever experience
> the rendering corruption again, I think this is a worthy patch as it
> does not happen on hotpath of the program anyway.
>
I believe the issue exists, but it is not solved correctly.
For example using cresize(w, h) before ttynew would do an ioctl() on a
non-existing fd.
Using st -e lynx and the usleep delay it will initialize with the default dimensions.
Not all the space in the terminal window is used, but no output is garbled here.
This is on OpenBSD -current.
What exact system do you use?
--
Kind regards,
Hiltjo
Received on Fri Jun 05 2020 - 23:09:32 CEST