- Dillo-dev - lists.mailman3.com

BUG: CCC corrupts memory when the Content-Type is wrong

by onepoint＠starurchin.org

On Wed, Dec 05, 2007 at 12:34:18PM -0300, Jorge Arellano Cid wrote: > The problem is reentrancy. Aha! I did some testing and noticed that calls to a_IO_ccc were sometimes nested. > This bug was produced because a CCC operation was reentering the > same chain after it was aborted. I added an extra check as a > workaround to avoid this situation and a comment that a more generic > solution should be designed. Hmm, perhaps the CCC Chainlink structures could have an aborted flag and the chaining functions could call the callbacks only if the link was not aborted? Also, I noticed that a_Chain_unlink is never used. Is that intentional? > Patch is in CVS. Problem solved. Yay! Regards, Jeremy Henty

Dec. 5, 2007

BUG: CCC corrupts memory when the Content-Type is wrong

by jcid＠dillo.org

Hi Jeremy, On Tue, Dec 04, 2007 at 02:03:07PM +0000, Jeremy Henty wrote: > > I am hoping that someone who understands Dillo's CCC better than I can > point me in the right direction and save me some time debugging this. Well, ehmmmm, I know CCC, but this was a hard-to-find bug! > Dillo almost always crashes if the HTML contains the stumbleupon.com > favicon link, ie.'<img src="http://www.stumbleupon.com/favicon.ico">'. > (This is very annoying as it makes Dillo unusable for one of my > favourite blogs.) If it doesn't crash right away then clicking Reload > crashes it. > > Examining core dumps with gdb suggests that memory is being corrupted, > and running dillo under valgrind shows that the CCC functions are > reading, writing and freeing already freed memory. (Details > attached.) > > A local HTTP server triggers the bug when it server returns a > Content-Type of "text/plain; charset=iso-8859-1". If I change it to > "image/x-icon" there is no corruption. Accessing the image directly > via a "file:" URL does not trigger the bug. (Strangely, using a local > HTTP server does not hit the bug until I open a second window on the > page, although sometimes it crashes first time.) > > I'm attaching a summary of the invalid memory accesses that valgrind > reported, and the entire valgrind log (which includes stack traces). > > Any suggestions for where I should start instrumenting the CCC to find > out why it is prematurely freeing pointers? The problem is reentrancy. The CCC handles parallelism and serves to propagate operations in a managed environment. This bug was produced because a CCC operation was reentering the same chain after it was aborted. I added an extra check as a workaround to avoid this situation and a comment that a more generic solution should be designed. Patch is in CVS. Please test and comment. Hopefully it will work. ;-) -- Cheers Jorge.-

Dec. 5, 2007

[patch] initialize mustQueueResize in Textblock constructor

by jcid＠dillo.org

On Wed, Dec 05, 2007 at 10:59:24AM +0100, Johannes Hofmann wrote: > Hello, > > this patch initializes mustQueueResize in the Textblock constructor. > Valgrind was complaining about a jump depending on uninitialized > memory. Committed. -- Cheers Jorge.-

Dec. 5, 2007

[patch] add missing style->ref ()

by jcid＠dillo.org

On Wed, Dec 05, 2007 at 10:57:04AM +0100, Johannes Hofmann wrote: > Hello, > > the tiny patch below adds a missing style->ref() which was causing > a lot of crashes on my system. Committed. -- Cheers Jorge.-

Dec. 5, 2007

[patch] initialize mustQueueResize in Textblock constructor

by Johannes.Hofmann＠gmx.de

Hello, this patch initializes mustQueueResize in the Textblock constructor. Valgrind was complaining about a jump depending on uninitialized memory. Cheers, Johannes diff --git a/dw/textblock.cc b/dw/textblock.cc --- a/dw/textblock.cc +++ b/dw/textblock.cc @@ -39,6 +39,7 @@ Textblock::Textblock (bool limitTextWidt line1Offset = 0; line1OffsetEff = 0; ignoreLine1OffsetSometimes = false; + mustQueueResize = false; /* * The initial sizes of lines and words should not be

Dec. 5, 2007

[patch] add missing style->ref ()

by Johannes.Hofmann＠gmx.de

Hello, the tiny patch below adds a missing style->ref() which was causing a lot of crashes on my system. cheers, Johannes diff --git a/dw/fltkui.cc b/dw/fltkui.cc --- a/dw/fltkui.cc +++ b/dw/fltkui.cc @@ -159,6 +159,7 @@ void FltkResource::setStyle (core::style this->style->unref (); this->style = style; + style->ref (); for (Iterator <ViewAndWidget> it = viewsAndWidgets->iterator (); it.hasNext(); ) {

Dec. 5, 2007

[patch] partial redraws - updated

by jcid＠dillo.org

Hi, On Tue, Dec 04, 2007 at 02:42:32PM +0100, Johannes Hofmann wrote: > Hi Jorge, > > > > > Committed! > > > > > > Thanks. Attached is another patch that has some improvements to that > > > code: > > > > Excellent. > > > > Committed. > > until now, just the Changlog has been commited, right? Right, my mistake. :-P Committed again. > > If not, even when I'm not an expert in dw2, I'd prefer to > > push forward development in that area and to commit what looks > > OK. Sebastian will always have the opportunity to comment/fix > > later. > > Regarding the scrollbars there is something fishy in the current > code that needs fixing: The scrollbars are created in the > FltkViewport constructor and they are drawn on the screen area > belonging to FltkViewport. However from fltk point of view they are > children of TopGroup. Yes, we know there's something fishy there because while in Germany we discovered scrollbars worked without the line that was supposedly drawing them! :-) > I think this is the reason why fltk replace() does not work > properly. > > However, my proposed fix is really quite radical and I would want to > wait for a comment from Sebastian before we head into that > direction. OK. > > As a matter of fact I'm working of improving table rendering > > now, and I already have some code that works. It's a complex area > > that needs some work but I hope to commit a patch this week. > > > > BTW, did you receive comments on the patch that lowered CPU > > consumption when gliding the mouse over pages full of links? > > No, but the change is not that dramatic, so I think it can be > applied. Do you need a patch against current cvs? Committed. -- Cheers Jorge.-

Dec. 4, 2007

BUG: CCC corrupts memory when the Content-Type is wrong

by onepoint＠starurchin.org

Heh, I sent the valgrind logs separately but they were so large they were held for moderation. This is just the summarised version. It lists the freed blocks that were accessed and the sequence of invalid accesses. It's clear that the CCC is accessing pointers after freeing them. Jeremy Henty Freed blocks: [1]: 0x4AB7910:(IOData_t *), freed by a_IO_ccc (IO.c:385) [2]: 0x4AB78C0:(ChainLink *Info), freed by a_IO_ccc (IO.c:386) [3]: 0x4AB5768:(ChainLink *Info), freed by a_Dpi_ccc (dpi.c:696) [4]: 0x4AB5718:(ChainLink *Info), freed by a_Capi_ccc (capi.c:536) [5]: 0x4AACE10:(capi_conn_t *), freed by a_Capi_ccc (capi.c:535) [6]: 0x4AACE68:(DilloUrl *), freed by a_Capi_ccc (capi.c:535) [7]: 0x4AB57B8:(dpi_conn_t *), freed by a_Dpi_ccc (dpi.c:695) [8]: 0x4AB5818:(Dstr *), freed by a_Dpi_ccc (dpi.c:695) [9]: 0x4AB5858:(Dstr_char_t *), freed by a_Dpi_ccc (dpi.c:672) [10]: 0x4AB7960:(Dstr *ds), freed by a_IO_ccc (IO.c:385) [11]: 0x4AB79A0:(Dstr_char_t *), freed by a_IO_ccc (IO.c:385) [1] read at IO_read (IO.c:194) [2] read at a_IO_ccc (IO.c:402) (many times) [3] read at a_IO_ccc (IO.c:402) (many times) [4] read at a_Capi_ccc (capi.c:569) [5] write at a_Capi_ccc (capi.c:570) [5] read at a_Capi_ccc (capi.c:572) [6] read at a_Url_cmp (url.c:442) [5] read at a_Capi_ccc (capi.c:574) [5] read at Capi_conn_unref (capi.c:132) (twice) [4] freed by a_Capi_ccc (capi.c:579) [3] read at a_Dpi_ccc (dpi.c:672) [7] read at Dpi_conn_free (dpi.c:124) [7] read at Dpi_conn_free (dpi.c:125) [8] read at dStr_free (dlib.c:291) [8] read at dStr_free (dlib.c:292) [9] freed by a_Dpi_ccc (dpi.c:672) [8] freed by a_Dpi_ccc (dpi.c:672) [7] freed by a_Dpi_ccc (dpi.c:672) [3] freed by a_Dpi_ccc (dpi.c:673) [1] read at IO_read (IO.c:194) (many times) [10] read at dStr_free (dlib.c:291) [1] read at dStr_free (dlib.c:292) [11] freed by a_IO_ccc (IO.c:404) [10] freed by a_IO_ccc (IO.c:404) [1] freed by a_IO_ccc (IO.c:404) [2] freed by a_IO_ccc (IO.c:405)

Dec. 4, 2007

BUG: CCC corrupts memory when the Content-Type is wrong

by onepoint＠starurchin.org

I am hoping that someone who understands Dillo's CCC better than I can point me in the right direction and save me some time debugging this. Dillo almost always crashes if the HTML contains the stumbleupon.com favicon link, ie.'<img src="http://www.stumbleupon.com/favicon.ico">'. (This is very annoying as it makes Dillo unusable for one of my favourite blogs.) If it doesn't crash right away then clicking Reload crashes it. Examining core dumps with gdb suggests that memory is being corrupted, and running dillo under valgrind shows that the CCC functions are reading, writing and freeing already freed memory. (Details attached.) A local HTTP server triggers the bug when it server returns a Content-Type of "text/plain; charset=iso-8859-1". If I change it to "image/x-icon" there is no corruption. Accessing the image directly via a "file:" URL does not trigger the bug. (Strangely, using a local HTTP server does not hit the bug until I open a second window on the page, although sometimes it crashes first time.) I'm attaching a summary of the invalid memory accesses that valgrind reported, and the entire valgrind log (which includes stack traces). Any suggestions for where I should start instrumenting the CCC to find out why it is prematurely freeing pointers? Jeremy Henty

Dec. 4, 2007

[patch] partial redraws - updated

by Johannes.Hofmann＠gmx.de

Hi Jorge, On Tue, Dec 04, 2007 at 10:05:25AM -0300, Jorge Arellano Cid wrote: > Hi Johannes, > > On Tue, Dec 04, 2007 at 11:03:01AM +0100, Johannes Hofmann wrote: > > On Mon, Dec 03, 2007 at 02:00:07PM -0300, Jorge Arellano Cid wrote: > > > On Sun, Dec 02, 2007 at 01:53:14PM +0100, Johannes Hofmann wrote: > > > > Hello, > > > > > > > > this patch fixes the problem pointed out by Sebastian regarding > > > > partial redraws and scrolling interference. > > > > It adds a new Region class that is implemented similarily > > > > as in dillo1 as a set of rectangles not containing each other. > > > > The conversion from canvas to view coordinates is now done at > > > > draw-time. > > > > > > Committed! > > > > Thanks. Attached is another patch that has some improvements to that > > code: > > Excellent. > > Committed. until now, just the Changlog has been commited, right? > > > * The case that multiple rectangles can be exactly combined to a larger > > one is quite likely (other than the comment in dillo1 suggests). > > It happens all the time, e.g. when an image is loaded piece by piece. > > The region code is improved to take advantage of this. > > ok. > > > * A minor bug also present in the dillo1 code is fixed: If a > > rectangle existing already in the region is contained in a newly > > added rectangle, we must not stop the loop. The new rectangle may > > contain other rectangles as well! > > ok. > > > * The actual drawing of a region now checks, whether a rectangle is > > actually visible at the moment. > > Does this help to reduce flickering? I don't think so, it just shortcuts drawing operations that are not visible anyway. > > > * The background is now cleared properly when doing a partial > > redraw. This fixes some screen corruption occuring when selecting / > > deselecting text. > > ok. > > Please excuse me for not having yet the time to review the > patch for the scrolling bars. Has Sebastian commented it to you? No, not yet. > > If not, even when I'm not an expert in dw2, I'd prefer to > push forward development in that area and to commit what looks > OK. Sebastian will always have the opportunity to comment/fix > later. Regarding the scrollbars there is something fishy in the current code that needs fixing: The scrollbars are created in the FltkViewport constructor and they are drawn on the screen area belonging to FltkViewport. However from fltk point of view they are children of TopGroup. I think this is the reason why fltk replace() does not work properly. However, my proposed fix is really quite radical and I would want to wait for a comment from Sebastian before we head into that direction. > > As a matter of fact I'm working of improving table rendering > now, and I already have some code that works. It's a complex area > that needs some work but I hope to commit a patch this week. > > BTW, did you receive comments on the patch that lowered CPU > consumption when gliding the mouse over pages full of links? No, but the change is not that dramatic, so I think it can be applied. Do you need a patch against current cvs? Cheers, Johannes > > -- > Cheers > Jorge.- > > _______________________________________________ > Dillo-dev mailing list > Dillo-dev(a)dillo.org > http://lists.auriga.wearlab.de/cgi-bin/mailman/listinfo/dillo-dev

Dec. 4, 2007