[squid-dev] [PATCH] assertion failed: Write.cc:41: "!ccb->active()"

2016-03-10 Thread Christos Tsantilas
Hi all, I am attaching two patches for this bug. One simple for squid-3.5 (t1 patch) and one more complex (t2 patch). The simple patch solve the bug for now, but may leave other similar bugs in squid. Bug description: - The client side and server side are finished - On server side the F

Re: [squid-dev] [PATCH] assertion failed: Write.cc:41: "!ccb->active()"

2016-03-10 Thread Alex Rousskov
On 03/10/2016 12:14 PM, Christos Tsantilas wrote: > I am attaching two patches for this bug. I will re-summarize the problem we are dealing with using higher-level concepts so that it is easier to grok what Christos is talking about: 1. Ftp::Client cannot deal with more than one FTP command at

Re: [squid-dev] [PATCH] assertion failed: Write.cc:41: "!ccb->active()"

2016-03-13 Thread Christos Tsantilas
Hi all, I made all of the fixes requested by Alex. Please see below for my comments. On 03/10/2016 11:35 PM, Alex Rousskov wrote: On 03/10/2016 12:14 PM, Christos Tsantilas wrote: if (master->serverState == fssHandleDataRequest) { +if (!master->userDataDone) { ... +

Re: [squid-dev] [PATCH] assertion failed: Write.cc:41: "!ccb->active()"

2016-03-13 Thread Eliezer Croitoru
I would like to respond on some of the things in the post related to V4. First 3.5 is very stable for a very long time now. It have couple bugs in it and it will be good to somehow make them gone but I must highlight couple things which might not be clear. Atomic patches are great and they sol

Re: [squid-dev] [PATCH] assertion failed: Write.cc:41: "!ccb->active()"

2016-03-13 Thread Amos Jeffries
On 14/03/2016 8:57 a.m., Christos Tsantilas wrote: > Hi all, > > I made all of the fixes requested by Alex. > Please see below for my comments. > > On 03/10/2016 11:35 PM, Alex Rousskov wrote: >> On 03/10/2016 12:14 PM, Christos Tsantilas wrote: >> >>> if (master->serverState == fssHandleDat

Re: [squid-dev] [PATCH] assertion failed: Write.cc:41: "!ccb->active()"

2016-03-14 Thread Christos Tsantilas
On 03/14/2016 06:17 AM, Amos Jeffries wrote: On 14/03/2016 8:57 a.m., Christos Tsantilas wrote: Hi all, I made all of the fixes requested by Alex. Please see below for my comments. On 03/10/2016 11:35 PM, Alex Rousskov wrote: On 03/10/2016 12:14 PM, Christos Tsantilas wrote: if (maste

Re: [squid-dev] [PATCH] assertion failed: Write.cc:41: "!ccb->active()"

2016-03-14 Thread Alex Rousskov
On 03/13/2016 01:57 PM, Christos Tsantilas wrote: > On 03/10/2016 11:35 PM, Alex Rousskov wrote: >> On 03/10/2016 12:14 PM, Christos Tsantilas wrote: >>> if (master->serverState == fssHandleDataRequest) { >>> +if (!master->userDataDone) { >> ... >>> +originDataDownloadAbor

Re: [squid-dev] [PATCH] assertion failed: Write.cc:41: "!ccb->active()"

2016-03-14 Thread Alex Rousskov
On 03/13/2016 10:17 PM, Amos Jeffries wrote: > On 14/03/2016 8:57 a.m., Christos Tsantilas wrote: >> On 03/10/2016 11:35 PM, Alex Rousskov wrote: >>> The above logic looks correct to me, but I feel like I am reading an >>> inside-out code. Please consider this instead: >> I did not follow your sug

Re: [squid-dev] [PATCH] assertion failed: Write.cc:41: "!ccb->active()"

2016-03-14 Thread Alex Rousskov
On 03/13/2016 10:17 PM, Amos Jeffries wrote: > * stopOriginWait() does not make sense in English. Not my expertise area, but it does make sense to me. "Wait" can be a noun as in "we had a long wait" (as suggested by Google). Would you prefer stopOriginWaiting() accompanied by s/originWaitInProgr

Re: [squid-dev] [PATCH] assertion failed: Write.cc:41: "!ccb->active()"

2016-03-14 Thread Christos Tsantilas
On 03/14/2016 06:33 PM, Alex Rousskov wrote: On 03/13/2016 01:57 PM, Christos Tsantilas wrote: On 03/10/2016 11:35 PM, Alex Rousskov wrote: On 03/10/2016 12:14 PM, Christos Tsantilas wrote: if (master->serverState == fssHandleDataRequest) { +if (!master->userDataDone) { ... +

Re: [squid-dev] [PATCH] assertion failed: Write.cc:41: "!ccb->active()"

2016-03-14 Thread Alex Rousskov
On 03/14/2016 02:58 PM, Christos Tsantilas wrote: > On 03/14/2016 06:33 PM, Alex Rousskov wrote: >> The only remaining doubt in my mind is the combination of delayedReply >> and fssHandleDataRequest state. The above code appears to assume that, >> in fssHandleDataRequest, delayedReply is either alw

Re: [squid-dev] [PATCH] assertion failed: Write.cc:41: "!ccb->active()"

2016-03-14 Thread Alex Rousskov
On 03/10/2016 02:35 PM, Alex Rousskov wrote: > Amos, do you want us to port take2 to v3.5? The take1 patch for v3.5 is > enough to fix the known assertion. Take2 fixes that assertion as well, > but it is bigger because it also fixes design problems that may lead to > other bugs in v3.5. Which one d

Re: [squid-dev] [PATCH] assertion failed: Write.cc:41: "!ccb->active()"

2016-03-14 Thread Amos Jeffries
On 15/03/2016 10:41 a.m., Alex Rousskov wrote: > On 03/10/2016 02:35 PM, Alex Rousskov wrote: >> Amos, do you want us to port take2 to v3.5? The take1 patch for v3.5 is >> enough to fix the known assertion. Take2 fixes that assertion as well, >> but it is bigger because it also fixes design problem

Re: [squid-dev] [PATCH] assertion failed: Write.cc:41: "!ccb->active()"

2016-03-15 Thread Christos Tsantilas
I applied the t4 patch to trunk as r14592 I am attaching the t4 patch for squid-3.5. I believe that we should apply this patch for this major reasons: - The (statefull) FTP protocol requires good coordination between client-side and server side. The pinned connections is one mechanism which h

Re: [squid-dev] [PATCH] assertion failed: Write.cc:41: "!ccb->active()"

2016-03-15 Thread Christos Tsantilas
On 03/15/2016 07:28 PM, Christos Tsantilas wrote: I applied the t4 patch to trunk as r14592 The t4 patch applied to squid-3.5 as r14001 I am attaching the t4 patch for squid-3.5. I believe that we should apply this patch for this major reasons: - The (statefull) FTP protocol requires go