Re: valgrind still unhappy despite expect changes

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?

  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.


-- 
  Cheers
  Jorge.-