Re: valgrind still unhappy despite expect changes

Jorge wrote:
> On Thu, Sep 01, 2011 at 10:56:19AM -0300, Jorge Arellano Cid wrote:
> > On Thu, Sep 01, 2011 at 03:38:58PM +0200, Johannes Hofmann wrote:
> > > On Thu, Sep 01, 2011 at 09:57:39AM -0300, Jorge Arellano Cid wrote:
> > > > On Wed, Aug 31, 2011 at 09:42:21PM +0000, corvid wrote:
> > > > > Jorge wrote:
> 
> > > > > [...]
> > > > > In this one
> > > > > http://starurchin.org/dillo/valgrind/7ea9fc809376ddf7dde2908e2ecf999aea274130.html
> > > > > at least, it looks pretty clear that the bw is gone.
> 
> 
> > > > [...]
> > > >   Does the invalid read apply to 'bw' or 'bw->nav_expect_url'?
> > > >   (I don't know valgrind's semantics on it)
> > > 
> > > I'd guess bw is no longer pointing to valid memory (i.e. has been
> > > free'd) and therefore reading the 4 byte nav_expect_url pointer
> > > causes the valgrind message.
> > 
> >   OK, this line gave light in that direction:
> > 
> > ==28991==  Address 0x6961a34 is 44 bytes inside a block of size 68 free'd
> > 
> >   ... and I observe that bw has 17 items, 17*4 = 68,
> >     and the expect URL is the 12th item, (12-1)*4 = 44.
> > 
> >   So it's an already freed 'bw' and not its expected URL.
> > 
> >   This starts to make sense...  ;-)
> 
>   BTW, why there are two different stack traces?
>   What does the second one mean with respect to the first?

The top one shows the invalid read, and the bottom shows how the
memory was freed that the top tried to read.

>   If  I  look the first one (in reading order), it's clear that a
> keyboard  event  closed  a  browser window cancelling the expect.
> This is, for instance, middle-click a link, hit Ctrl-W before the
> data   stream   starts   to  arrive  (while  dillo  displays  the
> "contacting host" message).
> 
>   In  my  machine  this  happens  in  the  expected order with no
> problems.  Can  anybody  reproduce  the  valgrind  log using this
> procedure?
> 
>   OTOH, analyzing the code:
> 
> AFAIS:
> 
>  * The only place where a bw is freed is in a_UIcmd_close_bw [3]:
>    (the only call to a_Bw_free() in the codebase).
>  * bw is valid at [1]
>  * The problematic a_Nav_cancel_expect_if_eq() is called from
>    inside the call chain at [2].
>  * No other a_UIcmd_close_bw() appears in the log, so how came
>    bw to be invalid then?
> 
> 
>      void a_UIcmd_close_bw(void *vbw)
>      {
>         BrowserWindow *bw = (BrowserWindow *)vbw;
>         UI *ui = BW2UI(bw);
>         CustTabs *tabs = ui->tabs();
>   1     Layout *layout = (Layout*)bw->render_layout;
>      
>         MSG("a_UIcmd_close_bw bw=%p\n", bw);
>   2     a_Bw_stop_clients(bw, BW_Root + BW_Img + BW_Force);
>         delete(layout);
>         if (tabs) {
>            tabs->remove_tab(ui);
>            if (tabs->num_tabs() == 0)
>               delete tabs->window();
>         }
>   3     a_Bw_free(bw);
>         MSG("a_UIcmd_close_bw freed(bw=%p)\n", bw);
>      }
> 
> 
>   Any help is appreciatted.

My guess is that it could be something like
first, one bw asks for a page, and that becomes conn->bw,
and then another bw starts also asking for the page, and
the first bw closes, invalidating the conn->bw, and then
the second bw closes and the cancel expect code tries to
use the old bw.

I'm sure it won't turn out to be exactly that, but when I
saw that there was a conn->bw, it seemed that there have
to be ways for a conn to outlast a bw.