On Thu, Sep 01, 2011 at 07:47:48PM +0000, corvid wrote:
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/7ea9fc809376ddf7dde2908e2ecf999aea27413... 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.
Thanks. Well, in that case we have a weird situation: Both stacks have: ==28991== by 0x8053F49: CustTabs::handle(int) (uicmd.cc:204) Which translates into a bw (window or tab) that's closed twice by using the keyboard. Strange as it seems, let's analyze it a bit: For __both_ cases: 1.- fltk is sending the SHORTCUT event for KEYS_CLOSE_TAB 2.- dillo receives it in CustTabs::handle() and gets the bw with: UI *ui = (UI*)wizard()->value(); BrowserWindow *bw = a_UIcmd_get_bw_by_widget(ui); (which means the ui is the current one and that the bw is in our valid bw list.) 3.- dillo calls a_UIcmd_close_bw() which: a) stops the clients b) tabs->remove_tab(ui) // This switches to another bw and removes the ui from wizard * Here the question is how can dillo find the ui and same bw, in the wizard as current, and in the valid bw list (after 3c), for a second time at 2.- ? c) a_Bw_free(bw) // the bw is removed from our valid list and freed. The only answer I can find for the above question is: The memory space for bw is reused with a new ui. This is: * create a bw (e.g. address 0x888). * close the bw, free the bw (0x888 is available again) * create a bw (malloc reuses address 0x888). * close the bw // Here we have the situation! * free the bw (0x888 is available again) which is compatible with: "Valgrind Bug254420 - memory pool tracking broken" http://bugs.kde.org/show_bug.cgi?id=254420 but, of course, I may be wrong...
[...] 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.
"bw" is a bit a misleading name, bacause each tab is a bw. OTOH, each tab has its own control panel and thus becomes a browser window object. In the case you propose there'd be two different bw.
>>>>>
Bottom line: This bug is currently holding the dillo-3.0 release. Given the analysis, that nobody knows how to reproduce it, that even if real, it doesn't do much harm, the situation is: if there's no news in a couple of days, the release will be made. -- Cheers Jorge.-