Re: [9fans] bug in rio: unhiding deleted windows

["Rudolf Sykora" <rudolf.sykora@gmail.com>]

2008/10/8 andrey mirtchovski <mirtchovski@gmail.com>:
> while looking around for information on the RC strangeness i triggered
> a bug with rio: if the last window on the "hidden" stack disappears
> _while_ the third mouse button menu is opened, then upon attempting to
> unhide the window without releasing the mouse button in between will
> result in a read addr fault in 'wunhide()'
>
> here's a stack trace and error message:
>
> acid: lstk()
> wunhide(h=0x1)+0x30 /sys/src/cmd/rio/rio.c:1099
>        w=0x1768e0
>        i=0x0
> unhide(h=0x6)+0x27 /sys/src/cmd/rio/rio.c:1129
> button3menu()+0x97 /sys/src/cmd/rio/rio.c:686
> mousethread()+0x2c4 /sys/src/cmd/rio/rio.c:589
>        sending=0x0
>        scrolling=0x0
>        moving=0x0
>        winput=0xdaf40
>        xy=0x1f6
>        inside=0x1
>        tmp=0x0
>        w=0xdaf40
>        oin=0x929e0
>        band=0x1
>        r=0xfefefefe
>
> to reproduce:
>
> open a rio window
> run "window -hide 'sleep 10'"
> click the third mouse button (to bring the "New" menu)
> wait 10 seconds (you will know that 10 seconds have passed if you move
> the mouse over the last hidden window and the highlighted text appears
> to be gibberish)
> unhide the last hidden window by releasing the third mouse button
> while pointing at it in the menu
>
> if you simply release the third mouse button and bring the menu up
> again you will see that the window has disappeared. it is only a bug
> when rio isn't given a chance to refresh the third button menu.
>
> cheers: andrey
>
>

Although I have somehow realized that plan9 probably can't be of much
use to me, I decided to at least somehow contribute to it (before I
abandon it?) and tried to investigate what could be done with respect
to the bug described above.

Well, I am not a programmer...

In order to at least not allow rio to break completely in the
situation, maybe this could be added to /sys/src/cmd/rio/rio.c:

: ruda; diff rio.c rio.c_orig
683c683
< 		if(hidden[i] != nil) unhide(i);
---
> 		unhide(i);
: ruda;

This usually helps, but it neither solves much, nor it is 100 % sure
it saves you. The problem, as I see it now, is more difficult.

Rio apparently stores a list of hidden windows in
    Window *hidden[100]
Should this list couldn't change when playing with rio's menus, things
would be easy. But it can (the hidden window in our example exits).
And, generally, it can possibly change in any way. To underline this
fact, note that even on one single line like mine
    if(hidden[i] != nil) unhide(i);
the hidden[i] and unhide(i) can possibly pertain to completely
different windows if we somehow manage to terminate some and create
others in the meanwhile (I suppose this can happen, the code can be
interrupted and there are not locks).

[In the described example, what really happens is this. We create the
hidden window
that will exit in 10 s. Before it does, we press the 3rd button and
(keeping it down) we see the window item in the menu. By this time we
are in menuhit() function of libdraw. To this function pointers to
hidden windows' labels are passed (via Menu). When this function is
entered, it draws the physical menu on the screen with correct names.
When our hidden window exits, the menuhit() doesn't know anything
about it. Since it has drawn the physical menu on the function
entrance, when the label pointers still pointed somewhere, the screen
picture stays intact. When we move our mouse above the item which
corresponded to the hidden window, the drawing function needs to
redraw the field (to change the colours). At this moment it reads the
(now non-valid) pointer again and draws rubbish. As soon as we move
back to another item of the menu (still keeping the 3rd button down),
the drawing function redraws the field with something (a picture) it
used (and remembers) from the time the drawing function was first
entered. That's why the field looks incorrect only when being above
the field... If we release the button above the field of the now
nonexistent window, the unhide() (or wunhide()) function in rio.c
tries to unhide the window, and it gets essentially stuck. This is
where I try to intervene with my if condition.]

[In order to not see any rubbish, but a (stale; can't be much
better---the menu is rather detached from rio) menu, I thought it
would suffice to make a copy of the labels before calling menuhit()
from button3menu() in rio.c. Something like changing, close to the
beginning of button3menu(),
menu3str[i+Hidden] = hidden[i] -> label
to
menu3str[i+Hidden] = strdup(hidden[i] -> label)
and releasing the memory upon exit from button3menu().
However, then I realized, that the windows can change at any time, as
I tried to explain above, so that any such attempt is generally wrong
from the very beginning, and things have to be repaired more
thoroughly].

In this view I feel the whole thing is just flawed. I appologise to
the authors, if I am wrong. But it definitely disappointed me, along
with limitations like that 100 static allocation size (and there are
more). I'd like to see a system _without_ any such static things in
spite of the fact you probably will never have so many hidden windows.
I consider it a good & only right way how it should be done today.

If someone could check it... :)
Best regards
Ruda