Re: client_side and comm_close

Alex Rousskov wrote:
> On Thu, 2008-04-17 at 20:13 +0300, Tsantilas Christos wrote:
>> Alex Rousskov wrote:
>>> In reply to comment #8 for bug #2309
>>> comm_read assertion failed: "ccb->active == false"
>>> http://www.squid-cache.org/bugs/show_bug.cgi?id=2309
>>>
>>>> Note to the one looking into this bug: The assert happens if a
>>>> comm_read() is called while there already is a comm_read() pending.
>>> Yes.
>>>
>>>> I would suspect the half closed client check has been wrongly
>>>> adopted to call comm_read where it used commSetSelect periodically
>>>> before..
>>> d
>>> The above may be true, but there are other conditions causing the same
>>> assertion in my tests, with and without the pending comm_close patch
>>> from Christos.
>>>
>> As I can understand the problem does not exist only in squid3.0 or only
>> in squid3-trunk.
>>
>> The main problem with the client_side code is that the comm_read done by
>> the ConnStateData class, the comm_writes by the ClientSocketContext
>> class and comm_close (and other comm operations ) by all. This makes
>> very difficult to manage the comm related problems in client_side code....
>
> Yes. If we decide to fix this, there will be a single object responsible
> for client connection management. Others will simply use its services as
> needed. Comm cannot be such an object because it is too low-level.
> Currently, the functionality is spread among many classes.
>
>>> We should decide whether to continue patching holes here and there or
>>> clean up the client_side*cc connection management mess for good. Should
>>> we continue to patch isolated v3.0 problems and cleanup v3.1? Or is this
>>> a v3.2 project? Or am I exaggerating the problems since common cases
>>> usually work fine?
>
> The question remains.

IMO, This should be queued as part of the 3.1 cleanups, but not as
urgently as some other issues.

The priorities as I see them now are:
  - immediately fixable and urgent bugs.
  - completing the dedicated 3.1 feature list (DEVEL is overdue).
  - cleanups for fixing more intrusive or design bugs (such as this, and
the 407 one).
  - shakedown for release cycle.

We have a few months of shakedown before anything like a good stable.
Plenty of time for cleanup not blocking new features.

Amos

-- 
Please use Squid 2.6.STABLE19 or 3.0.STABLE4