- Dillo-dev - lists.mailman3.com

[patch] fix cookies freezing dillo problem

by akemnade＠tzi.de

Hi Jorge, On Fri, 11 Apr 2008 15:34:49 -0400 Jorge Arellano Cid <jcid(a)dillo.org> wrote: > Note: What still puzzles me, is why accept() returns when > there's nothing to read. > Hmm, accept() returns when you call connect() on the other side. Many TCP server programs send stuff to the client before receiving anything from the client (like ssh servers, smtp servers) so there is nothing unusual about that. So, normally you have on cookies.dpi side: accept() returns after connect() on dillo side read() returns after write() on dillo side then on dillo side: read() returns after write() on cookies.dpi side. As far as I understand in the race condition case you have: dillo cookies.dpi connect(sock1,...) sock1=accept(....) connect(sock2,...) (via a_Dpi_send_blocking_cmd) read(sock1,...) write(sock2,...) (does not block, contents will be added to the recv queue) read(sock2,...) and now, both sides are waiting for data,.... Greetings Andreas Kemnade

April 12, 2008

Re: patch: file input

by corvid＠lavabit.com

Jorge wrote: > On Thu, Apr 10, 2008 at 04:13:56PM +0000, corvid wrote: > > If I read in a file, delete that file, and try to read it in again > > (uses E2EReload), the submit will send the (now empty) file because > > a_Capi_get_buf() finds the (now empty) entry in cache. > > So I'll have to find out where an a_Cache_entry_remove_by_url() should go. > > Yes. It looks like I'd have to figure out CCC for this, which would likely be more trouble than it's worth.

April 12, 2008

Re: patch: file input

by jcid＠dillo.org

On Fri, Apr 11, 2008 at 07:12:06PM +0000, corvid wrote: > Jorge wrote: > > On Fri, Apr 11, 2008 at 05:48:25PM +0000, corvid wrote: > > > BTW, do you think dillo should do anything if it gets a huge file from > > > a server but didn't know to expect the file to be huge ahead of time > > > (no content length, or chunked transfer encoding)? > > > > Yes, of course. > > I mean should it take some particular action (a dialog or something) > once it figures out that it's N megs into something big? Good question. ;) Considering that we can only flag the entry as HUGE after it has grown a lot, aborting the connection and removing the entry is not an option. For root URLs: stopping the transfer could be offered, but OTOH the user can press the "Stop" button at will, and if he hasn't, we can assume he wants to continue (i.e. leave dillo2 "as is"). For non root URLs (mainly images): stopping the transfer once the threshold has being met, could be. Especially if we consider that the pixmap size is much bigger than the data, it makes sense. If it's simple to implement, it may be worth the effort. BTW, if we have a user clicking an iso, or movie that's served as "text/plain", the internal content-type checker will catch it, abort the connection, and leave the option of "Save link as...". -- Cheers Jorge.-

April 12, 2008

[patch] fix cookies freezing dillo problem

by jcid＠dillo.org

Hi Andreas, On Fri, Apr 11, 2008 at 07:42:29PM +0200, Andreas Kemnade wrote: > Hi, > > after some debugging I found out > that dillo was frozen because it waits for input on the unix > socket from the cookies.dpi. > cookies.dpi itself blocks on the read() on the connection socket. > > If you kill dillo and stop dpid, the next time > the cookies.dpi starts, it finds the cookies files locked and terminates > immedediately. Then dpid starts the cookies.dpi again and again in an > infinite loop. > > In my theory, the freeze where both processes are hanging in read() > is caused when a connection is creating via > DEBUG_MSG(5, "Cookies.c: a_Cookies_set \n\t \"%s\" \n",cmd ); > a_Capi_dpi_send_cmd(NULL, NULL, cmd, "cookies", 1); > > but the data is not send yet and > /* Get the answer from cookies.dpi */ > dpip_tag = a_Dpi_send_blocking_cmd("cookies", cmd); > is called. > > I have attached a patch which uses a_Dpi_send_blocking_cmd(). > It seems to fix the problem. dillo has not been frozen again yet. > Maybe cookies.dpi should create the SockHandler only when there > is data to read (checking for it with select() first). I have the same patch here! (with a few minor additions). What was really misleading for me, was that the old dillo-0.8.6 has no problem at all with using the non-blocking: a_Capi_dpi_send_cmd(NULL, NULL, cmd, "cookies", 1); So AFAIS it ended being a race-condition problem. This is, the cookie_set command was requested before the cookie_get for the next image, but were answered in the reverse order in dillo2 but not in dillo1! Note: What still puzzles me, is why accept() returns when there's nothing to read. I'll commit to let it have more testing. Thanks for the help. > Index: src/cookies.c > =================================================================== > RCS file: /sfhome/cvs/dillo/dillo2/src/cookies.c,v > retrieving revision 1.4 > diff -u -r1.4 cookies.c > --- src/cookies.c 24 Feb 2008 22:15:53 -0000 1.4 > +++ src/cookies.c 11 Apr 2008 17:41:48 -0000 > @@ -155,6 +155,7 @@ > } > > for (i = 0; (cookie_string = dList_nth_data(cookie_strings, i)); ++i) { > + char *dpip_tag; > path = URL_PATH_(set_url); > snprintf(numstr, 16, "%d", URL_PORT(set_url)); > cmd = a_Dpip_build_cmd("cmd=%s cookie=%s host=%s path=%s port=%s", > @@ -162,7 +163,10 @@ > path ? path : "/", numstr); > > DEBUG_MSG(5, "Cookies.c: a_Cookies_set \n\t \"%s\" \n",cmd ); > - a_Capi_dpi_send_cmd(NULL, NULL, cmd, "cookies", 1); > + dpip_tag = a_Dpi_send_blocking_cmd("cookies", cmd); > + if (dpip_tag != NULL) { > + dFree(dpip_tag); > + } > dFree(cmd); > } > } -- Cheers Jorge.-

April 11, 2008

Re: patch: file input

by corvid＠lavabit.com

Jorge wrote: > On Fri, Apr 11, 2008 at 05:48:25PM +0000, corvid wrote: > > BTW, do you think dillo should do anything if it gets a huge file from > > a server but didn't know to expect the file to be huge ahead of time > > (no content length, or chunked transfer encoding)? > > Yes, of course. I mean should it take some particular action (a dialog or something) once it figures out that it's N megs into something big?

April 11, 2008

Re: patch: file input

by jcid＠dillo.org

On Fri, Apr 11, 2008 at 05:48:25PM +0000, corvid wrote: > Jorge wrote: > > On Fri, Apr 11, 2008 at 02:33:50PM +0000, corvid wrote: > > > Jorge wrote: > > > > Please also add a file size threshold, to avoid a dillo exit from an > > > > out of memory condition. > > > > > > Does something like this sound about right? > > > > Yes, but I'd add a WEB_RootUrl check: > > > > + if (entry->Flags & CA_HugeFile) { > > + AbortEntry = TRUE; > > + a_UIcmd_set_msg(Client_bw,"Huge file! (%dMB)", > > + entry->ExpectedSize / (1024*1024)); > > ++ if (ClientWeb->flags & WEB_RootUrl && > > + dStrcasecmp(URL_SCHEME(entry->Url), "file")) > > + OfferDownload = TRUE; > > + } > > I saw that the current code only did so for root URLs, but I didn't > realize it was intentional. It may look a bit strange, but a download offer on a URL the user didn't request directly may be puzzling. > BTW, do you think dillo should do anything if it gets a huge file from > a server but didn't know to expect the file to be huge ahead of time > (no content length, or chunked transfer encoding)? Yes, of course. -- Cheers Jorge.-

April 11, 2008

Re: patch: file input

by corvid＠lavabit.com

Jorge wrote: > On Fri, Apr 11, 2008 at 02:33:50PM +0000, corvid wrote: > > Jorge wrote: > > > Please also add a file size threshold, to avoid a dillo exit from an > > > out of memory condition. > > > > Does something like this sound about right? > > Yes, but I'd add a WEB_RootUrl check: > > + if (entry->Flags & CA_HugeFile) { > + AbortEntry = TRUE; > + a_UIcmd_set_msg(Client_bw,"Huge file! (%dMB)", > + entry->ExpectedSize / (1024*1024)); > ++ if (ClientWeb->flags & WEB_RootUrl && > + dStrcasecmp(URL_SCHEME(entry->Url), "file")) > + OfferDownload = TRUE; > + } I saw that the current code only did so for root URLs, but I didn't realize it was intentional. BTW, do you think dillo should do anything if it gets a huge file from a server but didn't know to expect the file to be huge ahead of time (no content length, or chunked transfer encoding)?

April 11, 2008

[patch] fix cookies freezing dillo problem

by akemnade＠tzi.de

Hi, after some debugging I found out that dillo was frozen because it waits for input on the unix socket from the cookies.dpi. cookies.dpi itself blocks on the read() on the connection socket. If you kill dillo and stop dpid, the next time the cookies.dpi starts, it finds the cookies files locked and terminates immedediately. Then dpid starts the cookies.dpi again and again in an infinite loop. In my theory, the freeze where both processes are hanging in read() is caused when a connection is creating via DEBUG_MSG(5, "Cookies.c: a_Cookies_set \n\t \"%s\" \n",cmd ); a_Capi_dpi_send_cmd(NULL, NULL, cmd, "cookies", 1); but the data is not send yet and /* Get the answer from cookies.dpi */ dpip_tag = a_Dpi_send_blocking_cmd("cookies", cmd); is called. I have attached a patch which uses a_Dpi_send_blocking_cmd(). It seems to fix the problem. dillo has not been frozen again yet. Maybe cookies.dpi should create the SockHandler only when there is data to read (checking for it with select() first). Greetings Andreas Kemnade

April 11, 2008

Re: patch: file input

by jcid＠dillo.org

On Fri, Apr 11, 2008 at 02:33:50PM +0000, corvid wrote: > Jorge wrote: > > Please also add a file size threshold, to avoid a dillo exit from an > > out of memory condition. > > Does something like this sound about right? Yes, but I'd add a WEB_RootUrl check: + if (entry->Flags & CA_HugeFile) { + AbortEntry = TRUE; + a_UIcmd_set_msg(Client_bw,"Huge file! (%dMB)", + entry->ExpectedSize / (1024*1024)); ++ if (ClientWeb->flags & WEB_RootUrl && + dStrcasecmp(URL_SCHEME(entry->Url), "file")) + OfferDownload = TRUE; + } > diff -pur dillo2/src/cache.c dillo2-cur/src/cache.c > --- dillo2/src/cache.c 2008-03-29 15:14:11.000000000 +0000 > +++ dillo2-cur/src/cache.c 2008-04-11 14:25:55.000000000 +0000 > @@ -861,16 +861,18 @@ static void Cache_process_queue(CacheEnt > } else { > Client_bw->redirect_level = 0; > } > - if (entry->Flags & CA_HugeFile) { > - a_UIcmd_set_msg(Client_bw,"Huge file! (%dMB)", > - entry->ExpectedSize / (1024*1024)); > - AbortEntry = OfferDownload = TRUE; > - } > } else { > /* For non root URLs, ignore redirections and 404 answers */ > if (entry->Flags & CA_Redirect || entry->Flags & CA_NotFound) > Client->Callback = Cache_null_client; > } > + if (entry->Flags & CA_HugeFile) { > + AbortEntry = TRUE; > + a_UIcmd_set_msg(Client_bw,"Huge file! (%dMB)", > + entry->ExpectedSize / (1024*1024)); > + if (dStrcasecmp(URL_SCHEME(entry->Url), "file")) > + OfferDownload = TRUE; > + } > > /* Set the client function */ > if (!Client->Callback) { > _______________________________________________ > Dillo-dev mailing list > Dillo-dev(a)dillo.org > http://lists.auriga.wearlab.de/cgi-bin/mailman/listinfo/dillo-dev -- Cheers Jorge.-

April 11, 2008

Re: patch: file input

by corvid＠lavabit.com

Jorge wrote: > Please also add a file size threshold, to avoid a dillo exit from an > out of memory condition. Does something like this sound about right?

April 11, 2008